[MM-54885] Improve resiliency of browser initialization logic #50
Conversation
|
|
||
| slog.Debug("connected to call") | ||
| close(rec.readyCh) | ||
| return fmt.Errorf("stop signal received while initializing client") |
There was a problem hiding this comment.
Just wondering when this might happen? (I mean practically)
There was a problem hiding this comment.
That's needed because if the browser continuously fails to start (line 169) we still want to exit properly when the global readyTimeout expires. Otherwise we'd be stuck there indefinitely.
| @@ -46,3 +50,35 @@ func checkOSRequirements() error { | |||
|
|
|||
| return nil | |||
| } | |||
|
|
|||
| func pollBrowserEvaluateExpr(ctx context.Context, expr string, interval, timeout time.Duration, stopCh chan struct{}) error { | |||
| if err := recorder.Stop(); err != nil { | ||
| slog.Error("failed to stop recorder", slog.String("err", err.Error())) | ||
| } |
There was a problem hiding this comment.
I wonder if we should surface these to the sysadmin somehow, or else they might get lost/ignored. I understand simply logging most of what's in the recorder (that's info you would go find when things go wrong), but this seems like something that might get missed.
There was a problem hiding this comment.
The job failed to start, that's a major problem. If stopping fails it's more of a minor issue at that point I am thinking.
streamer45
added
3: Reviews Complete
Summary
PR implements an additional layer of re-try logic in case of browser failures during initialization. This should cover both a failure navigating to the URL (e.g. network failure) as well as any subsequent failure to initialize the calls client (e.g. plugin JS assets fail to get downloaded).
I also ported a couple of improvements to the start/stop flow from #44 so that we can get this running without having to make breaking changes.
Ticket Link
https://mattermost.atlassian.net/browse/MM-54885