fix(router): map gRPC errors to proper HTTP codes (#31)

cmd/atenet/internal/app/router/errors.go

@@ -0,0 +1,86 @@
+//  Copyright 2026 Google LLC
+//
+//  Licensed under the Apache License, Version 2.0 (the "License");
+//  you may not use this file except in compliance with the License.
+//  You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+//  Unless required by applicable law or agreed to in writing, software
+//  distributed under the License is distributed on an "AS IS" BASIS,
+//  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+//  See the License for the specific language governing permissions and
+//  limitations under the License.
+
+package router
+
+import (
+	"fmt"
+
+	envoy_type "github.com/envoyproxy/go-control-plane/envoy/type/v3"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
+)
+
+// newReqError builds a reqError whose body is the formatted message and no
+// wrapped cause. Set the cause field directly when one is available.
+func newReqError(code envoy_type.StatusCode, format string, args ...any) *reqError {
+	return &reqError{
+		msg:        fmt.Sprintf(format, args...),
+		statusCode: int(code),
+	}
+}
+
+// actorNotFoundErr returns a 404 reqError identifying the missing actor.
+func actorNotFoundErr(actorID string) *reqError {
+	return newReqError(envoy_type.StatusCode_NotFound, "actor %q not found", actorID)
+}
+
+// invalidHostErr returns a 404 reqError explaining why the request host was
+// rejected. The cause is preserved for log inspection via Unwrap.
+func invalidHostErr(host string, cause error) *reqError {
+	return &reqError{
+		msg:        fmt.Sprintf("invalid host %q: %v", host, cause),
+		cause:      cause,
+		statusCode: int(envoy_type.StatusCode_NotFound),
+	}
+}
+
+// mapResumeError translates an ActorResumer error into a client-facing
+// reqError. It maps gRPC status codes to appropriate HTTP status codes and
+// short, human-readable bodies. The original error is preserved via Unwrap
+// so callers can still inspect it via errors.Is / errors.As when logging.
+//
+// Unrecognised errors collapse to 500 with a generic body to avoid leaking
+// server-side detail (stack traces, internal IDs) to clients.
+func mapResumeError(actorID string, err error) *reqError {
+	if err == nil {
+		return nil
+	}
+
+	var re *reqError
+	switch status.Code(err) {
+	case codes.NotFound:
+		re = actorNotFoundErr(actorID)
+	case codes.FailedPrecondition:
+		// Preserve the gRPC description for FailedPrecondition only: it carries
+		// actionable client-facing context (e.g. "no free workers available")
+		// and is not security-sensitive.
+		re = newReqError(envoy_type.StatusCode_ServiceUnavailable,
+			"actor %q unavailable: %s", actorID, status.Convert(err).Message())
+	case codes.Unavailable:
+		re = newReqError(envoy_type.StatusCode_ServiceUnavailable, "actor %q unavailable", actorID)
+	case codes.DeadlineExceeded:
+		re = newReqError(envoy_type.StatusCode_GatewayTimeout, "actor %q request timed out", actorID)
+	case codes.PermissionDenied:
+		re = newReqError(envoy_type.StatusCode_Forbidden, "actor %q access denied", actorID)
+	case codes.Unauthenticated:
+		re = newReqError(envoy_type.StatusCode_Unauthorized, "actor %q authentication required", actorID)
+	case codes.ResourceExhausted:
+		re = newReqError(envoy_type.StatusCode_TooManyRequests, "actor %q rate limited", actorID)
+	default:
+		re = newReqError(envoy_type.StatusCode_InternalServerError, "error resuming actor %q", actorID)
+	}
+	re.cause = err
+	return re
+}

cmd/atenet/internal/app/router/errors_test.go

@@ -0,0 +1,178 @@
+//  Copyright 2026 Google LLC
+//
+//  Licensed under the Apache License, Version 2.0 (the "License");
+//  you may not use this file except in compliance with the License.
+//  You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+//  Unless required by applicable law or agreed to in writing, software
+//  distributed under the License is distributed on an "AS IS" BASIS,
+//  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+//  See the License for the specific language governing permissions and
+//  limitations under the License.
+
+package router
+
+import (
+	"errors"
+	"testing"
+
+	envoy_type "github.com/envoyproxy/go-control-plane/envoy/type/v3"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
+)
+
+func TestNewReqError(t *testing.T) {
+	t.Parallel()
+
+	err := newReqError(envoy_type.StatusCode_BadRequest, "actor %q is %s", "abc", "bad")
+	if err == nil {
+		t.Fatal("newReqError returned nil")
+	}
+	if err.statusCode != int(envoy_type.StatusCode_BadRequest) {
+		t.Errorf("statusCode = %d, want %d", err.statusCode, envoy_type.StatusCode_BadRequest)
+	}
+	if got, want := err.Error(), `actor "abc" is bad`; got != want {
+		t.Errorf("Error() = %q, want %q", got, want)
+	}
+}
+
+func TestActorNotFoundErr(t *testing.T) {
+	t.Parallel()
+
+	err := actorNotFoundErr("ctr6")
+	if err.statusCode != int(envoy_type.StatusCode_NotFound) {
+		t.Errorf("statusCode = %d, want %d", err.statusCode, envoy_type.StatusCode_NotFound)
+	}
+	if got, want := err.Error(), `actor "ctr6" not found`; got != want {
+		t.Errorf("Error() = %q, want %q", got, want)
+	}
+}
+
+func TestInvalidHostErr(t *testing.T) {
+	t.Parallel()
+
+	cause := errors.New("missing suffix")
+	err := invalidHostErr("foo.example.com", cause)
+
+	if err.statusCode != int(envoy_type.StatusCode_NotFound) {
+		t.Errorf("statusCode = %d, want %d", err.statusCode, envoy_type.StatusCode_NotFound)
+	}
+	if got, want := err.Error(), `invalid host "foo.example.com": missing suffix`; got != want {
+		t.Errorf("Error() = %q, want %q", got, want)
+	}
+	if !errors.Is(err, cause) {
+		t.Errorf("errors.Is(err, cause) = false, want true (cause should be wrapped for logging)")
+	}
+}
+
+func TestMapResumeError(t *testing.T) {
+	t.Parallel()
+
+	const actorID = "ctr6"
+
+	tests := []struct {
+		name     string
+		err      error
+		wantCode envoy_type.StatusCode
+		wantBody string
+	}{
+		{
+			name:     "NotFound maps to 404",
+			err:      status.Error(codes.NotFound, "actor not found"),
+			wantCode: envoy_type.StatusCode_NotFound,
+			wantBody: `actor "ctr6" not found`,
+		},
+		{
+			name:     "FailedPrecondition maps to 503 and preserves desc",
+			err:      status.Error(codes.FailedPrecondition, "no free workers available"),
+			wantCode: envoy_type.StatusCode_ServiceUnavailable,
+			wantBody: `actor "ctr6" unavailable: no free workers available`,
+		},
+		{
+			name:     "Unavailable maps to 503",
+			err:      status.Error(codes.Unavailable, "control-plane down"),
+			wantCode: envoy_type.StatusCode_ServiceUnavailable,
+			wantBody: `actor "ctr6" unavailable`,
+		},
+		{
+			name:     "DeadlineExceeded maps to 504",
+			err:      status.Error(codes.DeadlineExceeded, "context deadline exceeded"),
+			wantCode: envoy_type.StatusCode_GatewayTimeout,
+			wantBody: `actor "ctr6" request timed out`,
+		},
+		{
+			name:     "PermissionDenied maps to 403",
+			err:      status.Error(codes.PermissionDenied, "denied"),
+			wantCode: envoy_type.StatusCode_Forbidden,
+			wantBody: `actor "ctr6" access denied`,
+		},
+		{
+			name:     "Unauthenticated maps to 401",
+			err:      status.Error(codes.Unauthenticated, "no creds"),
+			wantCode: envoy_type.StatusCode_Unauthorized,
+			wantBody: `actor "ctr6" authentication required`,
+		},
+		{
+			name:     "ResourceExhausted maps to 429",
+			err:      status.Error(codes.ResourceExhausted, "quota"),
+			wantCode: envoy_type.StatusCode_TooManyRequests,
+			wantBody: `actor "ctr6" rate limited`,
+		},
+		{
+			name:     "unknown gRPC code maps to 500 without leaking desc",
+			err:      status.Error(codes.Internal, "stack trace: foo bar"),
+			wantCode: envoy_type.StatusCode_InternalServerError,
+			wantBody: `error resuming actor "ctr6"`,
+		},
+		{
+			name:     "non-gRPC error maps to 500 without leaking message",
+			err:      errors.New("raw error with secret"),
+			wantCode: envoy_type.StatusCode_InternalServerError,
+			wantBody: `error resuming actor "ctr6"`,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			got := mapResumeError(actorID, tc.err)
+			if got == nil {
+				t.Fatal("mapResumeError returned nil")
+			}
+			if got.statusCode != int(tc.wantCode) {
+				t.Errorf("statusCode = %d, want %d", got.statusCode, tc.wantCode)
+			}
+			if got.Error() != tc.wantBody {
+				t.Errorf("Error() = %q, want %q", got.Error(), tc.wantBody)
+			}
+			if !errors.Is(got, tc.err) {
+				t.Errorf("errors.Is(result, original) = false, want true (original must be preserved for logs)")
+			}
+		})
+	}
+}
+
+func TestMapResumeError_NilError(t *testing.T) {
+	t.Parallel()
+
+	// Guard against accidental nil-error calls. Returning nil keeps the
+	// happy path explicit at callsites instead of constructing a bogus 500.
+	if got := mapResumeError("ctr6", nil); got != nil {
+		t.Errorf("mapResumeError(_, nil) = %v, want nil", got)
+	}
+}
+
+// Ensures mapResumeError result satisfies the reqError contract so the
+// existing handleRequestHeaders branch (errors.As(err, &reqErr)) keeps working.
+func TestMapResumeError_IsReqError(t *testing.T) {
+	t.Parallel()
+
+	err := mapResumeError("x", status.Error(codes.NotFound, "x"))
+	var reqErr *reqError
+	if !errors.As(err, &reqErr) {
+		t.Fatalf("errors.As(*reqError) = false, want true; err type = %T", err)
+	}
+}

cmd/atenet/internal/app/router/extproc.go

@@ -124,8 +124,7 @@ func (s *ExtProcServer) handleRequestHeaders(
 
 	actorID, err := parseActorID(metadata.host)
 	if err != nil {
-		// Host is invalid, respond with 404.
-		return nil, metadata, "", notFoundErr
+		return nil, metadata, "", invalidHostErr(metadata.host, err)
 	}
 
 	slog.InfoContext(ctx, "ResumeActor", slog.String("actorID", actorID))
@@ -137,12 +136,13 @@ func (s *ExtProcServer) handleRequestHeaders(
 		slog.Any("err", err))
 
 	if err != nil {
-		return nil, metadata, "", fmt.Errorf("error resuming actor %s: %w", actorID, err)
+		return nil, metadata, "", mapResumeError(actorID, err)
 	}
 
 	workerIP := actor.GetAteomPodIp()
 	if ip := net.ParseIP(workerIP); ip == nil {
-		return nil, metadata, "", fmt.Errorf("actor %q did not have a valid IP %q", actorID, workerIP)
+		return nil, metadata, "", newReqError(envoy_type.StatusCode_InternalServerError,
+			"actor %q routing failed", actorID)
 	}
 
 	// TODO(bowei) -- handle more than port 80 on the actor.

cmd/atenet/internal/app/router/extproc_out.go

@@ -15,22 +15,23 @@
 package router
 
 import (
-	"errors"
-
 	corev3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
 	extproc "github.com/envoyproxy/go-control-plane/envoy/service/ext_proc/v3"
 	extprocv3 "github.com/envoyproxy/go-control-plane/envoy/service/ext_proc/v3"
 	envoy_type "github.com/envoyproxy/go-control-plane/envoy/type/v3"
 )
 
+// reqError carries an HTTP-mappable status code and a client-safe message.
+// The underlying cause (if any) is preserved via Unwrap so logs can inspect
+// the full chain without leaking server-side detail into the response body.
 type reqError struct {
-	error
+	msg        string
+	cause      error
 	statusCode int
 }
 
-var (
-	notFoundErr = &reqError{error: errors.New("not found"), statusCode: int(envoy_type.StatusCode_NotFound)}
-)
+func (e *reqError) Error() string { return e.msg }
+func (e *reqError) Unwrap() error { return e.cause }
 
 func addAuthorityMutation(auth string, mut *extprocv3.HeaderMutation) {
 	mut.SetHeaders = append(mut.SetHeaders,