Replace panic calls with proper error propagation#530
Open
swell-agent wants to merge 2 commits intowavesplatform:mainfrom
Open
Replace panic calls with proper error propagation#530swell-agent wants to merge 2 commits intowavesplatform:mainfrom
swell-agent wants to merge 2 commits intowavesplatform:mainfrom
Conversation
Replace all panic() calls with structured error handling using buffered channels and select-based multiplexing. Errors from goroutines are now propagated to callers instead of crashing the process. Key changes: - Add ServeErr() channel to API and BotAPI for HTTP server error reporting - Convert runMessagingClients/Services to return error channels - Use FanInCtx for composing error sources with proper context cancellation - Fix nil pointer dereference in vault.go when login fails with context.Canceled - Ensure scheduler shutdown runs on all exit paths in bot binaries - Log shutdown reason on service failure in nodemon - Simplify statementLogWrapper.String() and BaseTargetAlert.Message() Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
alexeykiselev
requested changes
Mar 24, 2026
cmd/bots/discord/discord.go
Outdated
| responseChan chan pair.Response, | ||
| ) { | ||
| ) <-chan error { | ||
| errCh := make(chan error, 2) //nolint:mnd // two goroutines |
Collaborator
There was a problem hiding this comment.
Please, remove this directive as suggested by golangci-lint.
|
|
||
| func TestBotAPIServeErrChannel(t *testing.T) { | ||
| t.Parallel() | ||
| logger := slog.New(slog.NewTextHandler(io.Discard, nil)) |
Collaborator
There was a problem hiding this comment.
Please, fix the linter issue as suggested.
cmd/bots/telegram/telegram.go
Outdated
| pairResponse chan<- pair.Response, | ||
| ) { | ||
| ) <-chan error { | ||
| errCh := make(chan error, 2) //nolint:mnd // two goroutines |
Collaborator
There was a problem hiding this comment.
Remove this directive because it's unused.
pkg/api/api_nopanic_test.go
Outdated
|
|
||
| func TestAPIServeErrChannel(t *testing.T) { | ||
| t.Parallel() | ||
| logger := slog.New(slog.NewTextHandler(io.Discard, nil)) |
Collaborator
There was a problem hiding this comment.
Please, fix linter issue as suggested.
Remove unused //nolint:mnd directives in discord and telegram bots. Replace slog.NewTextHandler(io.Discard, nil) with slog.DiscardHandler in test files.
nickeskov
reviewed
Mar 25, 2026
Comment on lines
+61
to
+65
| if loginErr != nil { | ||
| if !errors.Is(loginErr, context.Canceled) { | ||
| logger.Error("Unable to authenticate to Vault, stopping token renewal", attrs.Error(loginErr)) | ||
| } | ||
| return |
Collaborator
There was a problem hiding this comment.
Do exponential backoff with maximum backoff limit, not return. return here just stops the renew cycle.
| logger.Error("Unable to start managing token lifecycle", attrs.Error(tokenErr)) | ||
| panic(tokenErr) | ||
| logger.Error("Unable to start managing token lifecycle, stopping token renewal", attrs.Error(tokenErr)) | ||
| return |
Collaborator
There was a problem hiding this comment.
Do exponential backoff with maximum backoff limit, not return. return here just stops the renew cycle.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #409
panic()calls with structured error handling using buffered channels andselect-based multiplexingServeErr()channel toAPIandBotAPIfor HTTP server error reportingrunMessagingClients/runMessagingServicesto return error channelsFanInCtxwith proper context cancellation for composing error sources in nodemonvault.gowhen login fails withcontext.CanceledstatementLogWrapper.String()andBaseTargetAlert.Message()error handlingTest plan
makecompletes without errorspaniccalls remain in the codebaseapi_nopanic_test.go,alerts_nopanic_test.go🤖 Generated with Claude Code