- Notifications
You must be signed in to change notification settings - Fork928
fix: stop logging session shutdown as warning#12922
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This stack of pull requests is managed by Graphite.Learn more about stacking. Join@spikecurtis and the rest of your teammates on |
| @@ -156,7 +157,7 @@ func (api *API) workspaceAgentRPC(rw http.ResponseWriter, r *http.Request) { | |||
| ctx=tailnet.WithStreamID(ctx,streamID) | |||
| ctx=agentapi.WithAPIVersion(ctx,version) | |||
| err=agentAPI.Serve(ctx,mux) | |||
| iferr!=nil { | |||
| iferr!=nil&&!xerrors.Is(err,yamux.ErrSessionShutdown)&&!xerrors.Is(err,io.EOF){ | |||
| logger.Warn(ctx,"workspace agent RPC listen error",slog.Error(err)) | |||
| _=conn.Close(websocket.StatusInternalError,err.Error()) | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Should you still call close even on these errors?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I don't think so. The netConn is closed via adefer, which closes the underlying websocket --- the only difference is that it will sendNormalClosure rather thanInternalError, which seems fine for these error types.
Uh oh!
There was an error while loading.Please reload this page.
A customer hit like 200k of ErrSessionShutdown, which just dupes any errors we would have generated when shutting down the session for e.g. Ping failures.