diff --git a/packages/api/internal/orchestrator/create_instance.go b/packages/api/internal/orchestrator/create_instance.go index 46102e9aad..577da40e9a 100644 --- a/packages/api/internal/orchestrator/create_instance.go +++ b/packages/api/internal/orchestrator/create_instance.go @@ -272,6 +272,19 @@ func (o *Orchestrator) CreateSandbox( "please visit 'https://e2b.dev/docs/billing'", totalConcurrentInstances), Err: fmt.Errorf("team '%s' has reached the maximum number of instances (%d)", team.ID, totalConcurrentInstances), } + case errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded): + // The caller left (client disconnect or its deadline) while the + // reservation was being taken. That is the client leaving, not a + // server failure: don't log it at ERROR. The access-log/metrics + // middleware already maps the request to 499/408 by the cancel + // cause, so this never reaches the client as a 500. + logger.L().Warn(ctx, "sandbox reservation cut by the caller", logger.WithSandboxID(sandboxID), zap.Error(err)) + + return sandbox.Sandbox{}, &api.APIError{ + Code: http.StatusInternalServerError, + ClientMsg: fmt.Sprintf("Failed to create sandbox: %s", err), + Err: err, + } default: logger.L().Error(ctx, "failed to reserve sandbox for team", logger.WithSandboxID(sandboxID), zap.Error(err)) diff --git a/packages/api/internal/orchestrator/placement/placement.go b/packages/api/internal/orchestrator/placement/placement.go index 41ff2bd5ea..b97b8fea46 100644 --- a/packages/api/internal/orchestrator/placement/placement.go +++ b/packages/api/internal/orchestrator/placement/placement.go @@ -270,15 +270,28 @@ func placeSandbox( return PlacementResult{}, SandboxCreateError{Attempts: attempt + 1, LastErr: err} } + // A create cut by its own context (the caller cancelled or its + // deadline fired) says nothing about the node: it is the client + // leaving, not a node that refused the sandbox. Don't exclude it, + // don't count it as a failure sample, and don't log it at ERROR — + // doing so pollutes the ERROR stream and inflates createFails with + // client behaviour. Still Skip to drop the in-progress reservation, + // still count the attempt so deadline() classifies the timeout, and + // end with the caller through deadline(). + if createCut { + failedNode.PlacementMetrics.Skip(sbxRequest.GetSandbox().GetSandboxId()) + logger.L().Warn(ctx, "Sandbox create cut by the caller before it completed", logger.WithSandboxID(sbxRequest.GetSandbox().GetSandboxId()), logger.WithTemplateID(sbxRequest.GetSandbox().GetTemplateId()), logger.WithBuildID(sbxRequest.GetSandbox().GetBuildId()), logger.WithNodeID(failedNode.ID), zap.Int("attempt", attempt+1), zap.Error(utils.UnwrapGRPCError(err))) + lastCreateErr = err + attempt++ + + return deadline() + } nodesExcluded[failedNode.ID] = struct{}{} failedNode.PlacementMetrics.Fail(sbxRequest.GetSandbox().GetSandboxId()) logger.L().Error(ctx, "Failed to create sandbox", logger.WithSandboxID(sbxRequest.GetSandbox().GetSandboxId()), logger.WithTemplateID(sbxRequest.GetSandbox().GetTemplateId()), logger.WithBuildID(sbxRequest.GetSandbox().GetBuildId()), logger.WithNodeID(failedNode.ID), zap.Int("attempt", attempt+1), zap.Error(utils.UnwrapGRPCError(err))) lastCreateErr = err attempt++ - if createCut { - return deadline() - } // The loop ends with the caller. if createDetached && ctx.Err() != nil { return answered() diff --git a/packages/api/internal/orchestrator/placement/placement_timeout_test.go b/packages/api/internal/orchestrator/placement/placement_timeout_test.go index 7facab943d..8e4e8660a5 100644 --- a/packages/api/internal/orchestrator/placement/placement_timeout_test.go +++ b/packages/api/internal/orchestrator/placement/placement_timeout_test.go @@ -159,6 +159,38 @@ func TestPlaceSandbox_HardFailureNotWrapped(t *testing.T) { require.Error(t, err) assert.False(t, result.TimedOut, "a live-context failure must not be reported as a timeout") assert.Nil(t, result.WarmedNode, "a live-context failure must not surface a warming node") + assert.EqualValues(t, 1, node.PlacementMetrics.FailsCount(), "a live-context failure must count against the node") +} + +// TestPlaceSandbox_CutCreateNotCountedAgainstNode verifies that a create cut by +// its own context (the caller left) is NOT counted as a node failure: the node +// keeps a zero createFails and is not excluded, because a client disconnect says +// nothing about the node's health. See issue #3683. +func TestPlaceSandbox_CutCreateNotCountedAgainstNode(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithCancel(t.Context()) + + node := nodemanager.NewTestNode("node-cut", api.NodeStatusReady, 0, 8) + // The create cancels the request context and then fails - a client that + // disconnected (or whose deadline fired) while the create was in flight. + node.SetSandboxClient(erroringClient(cancel, status.Error(codes.Canceled, "context canceled"))) + + result, err := PlaceSandbox( + ctx, + failIfCalled(t), + []*nodemanager.Node{node}, + node, + testSbxRequest("sbx-cut"), + CPURequirement{}, + false, + nil, + ) + + require.Error(t, err) + assert.True(t, result.TimedOut, "a cut create must be reported as a timeout") + assert.Zero(t, node.PlacementMetrics.FailsCount(), "a cut create must not count against the node") + assert.Zero(t, node.PlacementMetrics.InProgressCount(), "a cut create must drop its in-progress reservation") } // TestPlaceSandbox_ResourceExhaustedNotPinned verifies that a node which refused