From 0d9b3b087d86083561d10a60a28c26685a8c3791 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 7 Jun 2026 23:11:27 +0000 Subject: [PATCH] fix(purchases): distinguish not-found from DB errors in GetExecutionByID (#976) GetExecutionByID previously returned (nil, error) for a missing row, which caused getPurchaseDetails to map every DB failure - including real connection errors - to a 404 ClientError. Callers had no way to distinguish a missing row from an infrastructure failure. Change GetExecutionByID to return (nil, nil) when no row matches, following the sentinel pattern used by other store methods. Update getPurchaseDetails to return a 500 on real DB errors and a 404 only on the nil-execution sentinel. Update tests to mock the new contract. Closes #976 --- internal/api/handler_purchases.go | 8 +++++--- internal/api/handler_purchases_test.go | 15 ++++++++------- internal/config/store_postgres.go | 8 ++++++-- 3 files changed, 19 insertions(+), 12 deletions(-) diff --git a/internal/api/handler_purchases.go b/internal/api/handler_purchases.go index 72c1ae4e1..26d789d65 100644 --- a/internal/api/handler_purchases.go +++ b/internal/api/handler_purchases.go @@ -1268,11 +1268,13 @@ func (h *Handler) getPurchaseDetails(ctx context.Context, req *events.LambdaFunc execution, err := h.config.GetExecutionByID(ctx, executionID) if err != nil { - // Map any DB-level "not found" error to 404 so callers cannot infer - // whether a UUID exists by observing a 500 vs a 404 (issue #431). - return nil, NewClientError(404, "execution not found") + // Real DB failure (connection error, timeout, etc.) - surface as 500 + // so infrastructure issues are visible to operators and not silently + // masked as 404s (issue #976). + return nil, fmt.Errorf("failed to get execution: %w", err) } if execution == nil { + // GetExecutionByID returns (nil, nil) for a missing row (issue #976). return nil, NewClientError(404, "execution not found") } diff --git a/internal/api/handler_purchases_test.go b/internal/api/handler_purchases_test.go index 67e501126..b68634c37 100644 --- a/internal/api/handler_purchases_test.go +++ b/internal/api/handler_purchases_test.go @@ -1720,7 +1720,8 @@ func TestHandler_getPurchaseDetails_NotFound(t *testing.T) { mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil) mockAuth.grantAdmin() - mockStore.On("GetExecutionByID", ctx, "99999999-9999-9999-9999-999999999999").Return(nil, errors.New("not found")) + // GetExecutionByID returns (nil, nil) for a missing row (issue #976). + mockStore.On("GetExecutionByID", ctx, "99999999-9999-9999-9999-999999999999").Return(nil, nil) handler := &Handler{config: mockStore, auth: mockAuth} @@ -1771,10 +1772,10 @@ func TestHandler_getPurchaseDetails_NilExecution(t *testing.T) { } // TestHandler_getPurchaseDetails_NotFound_IsClientError is a focused -// regression for issue #431: a DB-level "not found" error from -// GetExecutionByID must produce a 404 ClientError, never a 500, so that -// unauthenticated callers cannot infer UUID existence by observing a -// status-code difference. +// regression for issue #431: a missing execution must produce a 404 ClientError, +// never a 500, so that callers cannot infer UUID existence by observing a +// status-code difference. GetExecutionByID returns (nil, nil) for a missing row +// (issue #976); the handler maps that sentinel to a 404. func TestHandler_getPurchaseDetails_NotFound_IsClientError(t *testing.T) { ctx := context.Background() mockStore := new(MockConfigStore) @@ -1792,8 +1793,8 @@ func TestHandler_getPurchaseDetails_NotFound_IsClientError(t *testing.T) { mockAuth.grantAdmin() const missingID = "dddddddd-dddd-dddd-dddd-dddddddddddd" - // Simulate the DB returning a generic error (as pgx does on a missing row). - mockStore.On("GetExecutionByID", ctx, missingID).Return(nil, errors.New("no rows in result set")) + // GetExecutionByID returns (nil, nil) for a missing row (issue #976). + mockStore.On("GetExecutionByID", ctx, missingID).Return(nil, nil) handler := &Handler{config: mockStore, auth: mockAuth} req := &events.LambdaFunctionURLRequest{ diff --git a/internal/config/store_postgres.go b/internal/config/store_postgres.go index 447b031f1..88bfc3616 100644 --- a/internal/config/store_postgres.go +++ b/internal/config/store_postgres.go @@ -1125,7 +1125,11 @@ func (s *PostgresStore) GetPendingExecutionsTx(ctx context.Context, tx pgx.Tx) ( return scanExecutionRows(rows) } -// GetExecutionByID retrieves a purchase execution by execution ID +// GetExecutionByID retrieves a purchase execution by execution ID. +// Returns (nil, nil) when no row matches executionID so callers can cleanly +// distinguish "not found" (nil execution, nil error) from a real DB failure +// (nil execution, non-nil error). All callers must check the execution for +// nil before use (closes issue #976). func (s *PostgresStore) GetExecutionByID(ctx context.Context, executionID string) (*PurchaseExecution, error) { query := ` SELECT plan_id, execution_id, status, step_number, scheduled_date, @@ -1146,7 +1150,7 @@ func (s *PostgresStore) GetExecutionByID(ctx context.Context, executionID string } if len(executions) == 0 { - return nil, fmt.Errorf("execution not found: %s", executionID) + return nil, nil } return &executions[0], nil