patch: fx group behavior differs between lists and non-lists #133
Conversation
This file contains 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
- the nil value for `Cancels []func() `group:"cancels,flatten"` is discarded by fx (not used during dependency injection) - but the nil value for `Cancel func() `group:"cancels"` is used and not discarded - currently this triggers panics when a wrp handler returns a nil for `Cancel func() `group:"cancels"` - this bug wasn't caught because we weren't returning errors when fx's lifecycle Start/Stop encountered a panic - #124 fixes our fx lifecycle
denopink
commented
May 17, 2024
| @@ -266,6 +266,10 @@ func onStop(ws *websocket.Websocket, qos *qos.Handler, shutdowner fx.Shutdowner, | |||
| ws.Stop() | |||
| qos.Stop() | |||
| for _, c := range cancels { | |||
There was a problem hiding this comment.
currently this triggers panics when a wrp handler returns a nil for Cancel func() group:"cancels"
this bug wasn't caught because we weren't returning errors when fx's lifecycle Start/Stop encountered a panic
- patch: fx lifecycle panics should return an error while rolling back and shuting down #124 fixes our fx lifecycle (this pr needs to be merged first) and introduces lifecycle tests
denopink
added a commit
that referenced
this pull request
May 17, 2024
#133 must be merged first - Currently if a panic occurs during our fx lifecycle, we log the panic and then ignore it. - Instead, we want to return an error in order for fx to rollback and shutdown
…ween-list-nonlist
schmidtw
approved these changes
May 17, 2024
schmidtw
deleted the
denopink/patch/fx-group-behavior-differs-between-list-nonlist
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Cancels []func() group:"cancels,flatten"is discarded by fx (not used during dependency injection)Cancel func() group:"cancels"is used and not discardedCancel func() group:"cancels"