Skip to content

fix(envd): distinguish client-cancel from errors in process stream logging - #3647

Open
AdaAibaby wants to merge 1 commit into
e2b-dev:mainfrom
AdaAibaby:fix/envd-process-stream-cancel-logging
Open

AdaAibaby wants to merge 1 commit into
e2b-dev:mainfrom
AdaAibaby:fix/envd-process-stream-cancel-logging

Conversation

@AdaAibaby

Copy link
Copy Markdown
Contributor

Problem

When a client stops reading a server-streaming call, the handler returns context.Canceled, and the stream interceptor logged it at ERROR with error_code=2 (Unknown) — indistinguishable from a genuine failure.

Process.Start makes this routine and noisy. The process runs on its own context (context.Background()-derived procCtx), decoupled from the stream, so cancelling the stream does not kill the process — it runs to completion. Yet a client that fires a command and moves on (got its result, timed out, or a sibling task cancelled it) produces:

ERROR Process start (server stream end) error="context canceled" error_code=2 operation_id=17

This says nothing about why it was cancelled, and buries real errors at the same level. On short-lived sandboxes it is a constant source of scary-looking-but-benign ERROR lines.

Changes (observability only, no behaviour change)

  1. getErrDebugLogEvent picks the level by the error: nil→Debug, context.CanceledInfo (client went away, expected), context.DeadlineExceededWarn, everything else stays Error. Client cancellations stop sitting next to genuine failures.

  2. codeOf maps the context sentinels context.CanceledCodeCanceled and context.DeadlineExceededCodeDeadlineExceeded, instead of letting connect.CodeOf report them as Unknown(2). Applied at the three error_code emission sites so downstream aggregation is meaningful.

  3. The three <-ctx.Done() branches in process.Start now cancel with a cause that names the phase (before start / while streaming output / before end) and wraps context.Cause(ctx), so the "server stream end" line carries where and why the stream ended rather than a bare context canceled.

Testing

  • internal/logs/interceptor_test.go (new): level classification incl. wrapped errors (fmt.Errorf(... %w, context.Canceled)), and the codeOf mapping (canceled/deadline/plain/preserved connect code).
  • go build ./..., go vet, full internal/logs tests, and golangci-lint (v2, pinned) all clean.

Note

This is the server side of the picture. The client SDK is the party that actually knows why it cancelled (request timeout, sibling failure, caller exit, explicit kill); a companion SDK change to log that local cause and correlate via operation_id would complete the diagnosis, and can follow separately.

…gging

A client that stops reading a server-streaming call (Process.Start most
visibly) makes the handler return context.Canceled, and the stream
interceptor logged that at ERROR with error_code=2 (Unknown) — identical to
a real failure. On a short-lived sandbox this is routine: the client got
what it needed and moved on, or a sibling task/timeout cancelled it, while
the process keeps running to completion on its own context. The noisy line
was "Process start (server stream end) error=context canceled error_code=2",
which says nothing about why it was cancelled and drowns real errors.

Three changes, all observability, no behaviour change:

- getErrDebugLogEvent now picks the level by the error: nil->Debug,
  context.Canceled->Info (client went away, expected), DeadlineExceeded->
  Warn, everything else stays Error. Client cancellations stop being logged
  next to genuine failures.
- codeOf maps context.Canceled->CodeCanceled and DeadlineExceeded->
  CodeDeadlineExceeded instead of letting connect.CodeOf report them as
  Unknown(2), so error_code aggregates meaningfully. Used at the three
  error_code emission sites.
- The three <-ctx.Done() branches in process Start now cancel with a cause
  that names the phase (before start / while streaming / before end) and
  wraps context.Cause(ctx), so the end log carries why and where rather than
  a bare "context canceled".

Adds interceptor_test.go covering the level classification (incl. wrapped
errors) and the code mapping.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants