Pass irrecoverable context to ws handler #7154
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7154 +/- ##
=======================================
Coverage 41.25% 41.26%
=======================================
Files 2170 2170
Lines 190050 190109 +59
=======================================
+ Hits 78409 78451 +42
- Misses 105097 105114 +17
Partials 6544 6544
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
cmd/util/cmd/run-script/cmd.go
Outdated
| api := &api{ | ||
| chainID: chainID, | ||
| vm: vm, | ||
| ctx: ctx, | ||
| storageSnapshot: storageSnapshot, | ||
| } | ||
|
|
||
| irrCtx, _ := irrecoverable.WithSignaler(context.Background()) |
There was a problem hiding this comment.
it's not a good practice to ignore the error channel from irrecoverable contexts since that could end up ignoring an exception!
This is the preferred way to handle it
https://github.com/onflow/flow-go/pull/7155/files#diff-29dbb5e7e1c4ab893e43c26264cbe29048076a9bf049a466032f11c498572717R108-R116
There was a problem hiding this comment.
The problem here is that we do not have any parent irrecoverable context, just a regular context. Should we just log.Fatal().Err(err) in such a case?
There was a problem hiding this comment.
yea, log.Fatal() the the best alternative
There was a problem hiding this comment.
Done it. Can you make sure everything is ok in the new version?
illia-malachyn
force-pushed
the
illia-malachyn/7109-add-irrecoverable-ctx-to-ws-controller
branch
from
9516d3e to
a659b1b
Compare
|
Sorry, I rebased on top of master instead of merging by mistake, so I had to force-push. |
cmd/util/cmd/run-script/cmd.go
Outdated
| api := &api{ | ||
| chainID: chainID, | ||
| vm: vm, | ||
| ctx: ctx, | ||
| storageSnapshot: storageSnapshot, | ||
| } | ||
|
|
||
| irrCtx, errCh := irrecoverable.WithSignaler(context.Background()) | ||
| go func() { | ||
| err := modutil.WaitError(errCh, nil) |
There was a problem hiding this comment.
I use nil for the done channel. nil channel in the select statement is never selected.
peterargue
deleted the
illia-malachyn/7109-add-irrecoverable-ctx-to-ws-controller
branch
Closes #7109