Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/orchestrator/pkg/template/build/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ func (b *Builder) Build(ctx context.Context, paths storage.Paths, cfg config.Tem
e = errors.Join(e, ctx.Err())
}

e = builderrors.WrapContextAsUserError(e)
e = builderrors.WrapContextAsUserError(ctx, e)
if e != nil {
childSpan.RecordError(e, trace.WithAttributes(
telemetry.WithTemplateID(cfg.TemplateID),
Expand Down
24 changes: 17 additions & 7 deletions packages/orchestrator/pkg/template/build/builderrors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,10 @@ func IsUserError(err error) bool {
return phases.UnwrapPhaseBuildError(err) != nil
}

// WrapContextAsUserError wraps context.Canceled as a user error if no user error already exists.
// This ensures that user-initiated cancellations are tracked as user errors in metrics.
func WrapContextAsUserError(err error) error {
// WrapContextAsUserError wraps context errors as user errors only when the build-level
// context itself was canceled or timed out. This prevents internal child-context
// cancellations (e.g., envd init timeout) from being misclassified as user cancellations.
func WrapContextAsUserError(buildCtx context.Context, err error) error {
if err == nil {
return nil
}
Expand All @@ -38,13 +39,22 @@ func WrapContextAsUserError(err error) error {
return err
}

// If it's a canceled context, wrap it as a user error
if errors.Is(err, context.Canceled) {
// Only classify as user cancellation/timeout if the build-level context was actually done.
// Internal child-context cancellations should not be treated as user errors.
if buildCtx.Err() == nil {
return err
}

// If the build context was canceled AND the error chain contains context.Canceled,
// wrap it as a user cancellation. The double check ensures we don't discard
// unrelated errors (e.g., disk errors) that happen to coincide with context cancellation.
if buildCtx.Err() == context.Canceled && errors.Is(err, context.Canceled) {
return phases.NewPhaseBuildError(phases.PhaseMeta{}, ErrCanceled)
}

// If it's a timeout context, wrap it as a user error
if errors.Is(err, context.DeadlineExceeded) {
// If the build context deadline was exceeded AND the error chain contains
// context.DeadlineExceeded, wrap it as a user timeout.
if buildCtx.Err() == context.DeadlineExceeded && errors.Is(err, context.DeadlineExceeded) {
return phases.NewPhaseBuildError(phases.PhaseMeta{}, ErrTimeout)
}
Comment thread
AdaAibaby marked this conversation as resolved.

Expand Down
138 changes: 138 additions & 0 deletions packages/orchestrator/pkg/template/build/builderrors/errors_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
//go:build linux

package builderrors

import (
"context"
"errors"
"fmt"
"testing"

"github.com/e2b-dev/infra/packages/orchestrator/pkg/template/build/phases"
)

func TestWrapContextAsUserError_NilError(t *testing.T) {
t.Parallel()

ctx := context.Background()
if got := WrapContextAsUserError(ctx, nil); got != nil {
t.Errorf("expected nil, got %v", got)
}
}

func TestWrapContextAsUserError_AlreadyUserError(t *testing.T) {
t.Parallel()

ctx := context.Background()
userErr := phases.NewPhaseBuildError(phases.PhaseMeta{}, errors.New("user mistake"))
got := WrapContextAsUserError(ctx, userErr)
if !errors.Is(got, userErr) {
t.Errorf("expected original user error returned as-is, got %v", got)
}
}

func TestWrapContextAsUserError_InternalTimeout_NotMisclassified(t *testing.T) {
t.Parallel()

// Simulate: build context is still active, but error contains context.Canceled
// from an internal child-context timeout (e.g., WaitForEnvd).
buildCtx := context.Background() // not canceled

// This is what doRequestWithInfiniteRetries returns when a child context is canceled
internalErr := fmt.Errorf("%w with cause: %w", context.Canceled, errors.New("syncing took too long"))

got := WrapContextAsUserError(buildCtx, internalErr)

// Should NOT be classified as user error — the build context is still alive
if IsUserError(got) {
t.Errorf("internal timeout should not be classified as user error, got: %v", got)
}
// Should preserve the original error
if !errors.Is(got, internalErr) {
t.Errorf("expected original error preserved, got: %v", got)
}
}

func TestWrapContextAsUserError_UserCancellation(t *testing.T) {
t.Parallel()

// Simulate: user cancels the build → build context is canceled
buildCtx, cancel := context.WithCancel(context.Background())
cancel() // user canceled

// Error must contain context.Canceled (as it would in real code via errors.Join or wrapping)
someErr := fmt.Errorf("some operation failed: %w", buildCtx.Err())
got := WrapContextAsUserError(buildCtx, someErr)

if !IsUserError(got) {
t.Errorf("expected user error when build context is canceled, got: %v", got)
}
if !errors.Is(got, ErrCanceled) {
t.Errorf("expected ErrCanceled, got: %v", got)
}
}

func TestWrapContextAsUserError_BuildContextCanceled_ErrorUnrelated(t *testing.T) {
t.Parallel()

// Build context is canceled, but the error is unrelated (e.g., disk error).
// Should NOT be wrapped as user error.
buildCtx, cancel := context.WithCancel(context.Background())
cancel()

diskErr := errors.New("disk I/O error")
got := WrapContextAsUserError(buildCtx, diskErr)

if IsUserError(got) {
t.Errorf("unrelated error should not be classified as user error when build context is canceled, got: %v", got)
}
if !errors.Is(got, diskErr) {
t.Errorf("expected original error preserved, got: %v", got)
}
}

func TestWrapContextAsUserError_BuildDeadlineExceeded(t *testing.T) {
t.Parallel()

// Simulate: build context deadline exceeded
buildCtx, cancel := context.WithTimeout(context.Background(), 0)
defer cancel()
<-buildCtx.Done() // ensure it's expired

// Error must contain context.DeadlineExceeded (as it would in real code)
someErr := fmt.Errorf("some operation failed: %w", buildCtx.Err())
got := WrapContextAsUserError(buildCtx, someErr)

if !IsUserError(got) {
t.Errorf("expected user error when build context timed out, got: %v", got)
}
if !errors.Is(got, ErrTimeout) {
t.Errorf("expected ErrTimeout, got: %v", got)
}
}

func TestWrapContextAsUserError_InternalCanceledError_BuildContextAlive(t *testing.T) {
t.Parallel()

// The key regression test: error chain contains context.Canceled from a child
// context, but the build context is NOT canceled. Must NOT be treated as user error.
buildCtx := context.Background()

// Simulate WaitForEnvd timeout chain:
// child context canceled → doRequestWithInfiniteRetries wraps ctx.Err()
childCtx, childCancel := context.WithCancelCause(buildCtx)
childCancel(errors.New("syncing took too long"))

err := fmt.Errorf("failed to init new envd: %w",
fmt.Errorf("%w with cause: %w", childCtx.Err(), context.Cause(childCtx)))

got := WrapContextAsUserError(buildCtx, err)

if IsUserError(got) {
t.Errorf("internal child-context cancellation must not be classified as user error, got: %v", got)
}
// The original error should be preserved
if !errors.Is(got, context.Canceled) {
t.Errorf("original error chain should be preserved, got: %v", got)
}
}
Loading