fix(event-handler): stop recursive HTTP error handler dispatch

docs/features/event-handler/http.md

@@ -368,6 +368,8 @@ If you need to send custom headers or a different response structure/code, you c
 
 !!! tip "You can throw HTTP errors in your route handlers, middleware, or custom error handlers!"
 
+When a custom error handler throws an HTTP error, the router can pass it to another registered handler. Each handler runs at most once in that error-resolution chain. If dispatch would call a handler again, the router returns the HTTP error's built-in status and message instead. This also prevents cycles involving multiple handlers.
+
 === "index.ts"
 
     ```ts hl_lines="3 11"

packages/event-handler/src/http/Router.ts

@@ -556,43 +556,51 @@ class Router<TEnv extends Env = Env> {
   }
 
   /**
-   * Handles errors by finding a registered error handler or falling
-   * back to a default handler.
+   * Handles errors through registered handlers and built-in responses.
+   *
+   * Calls each registered handler at most once per resolution chain. If a
+   * handler throws an HTTP error that would revisit a handler, returns that
+   * error's built-in response.
    *
    * @param error - The error to handle
    * @param options - Error resolve options including request context and scope
-   * @returns A Response object with appropriate status code and error details
    */
   protected async handleError(
     error: Error,
     options: ErrorResolveOptions
   ): Promise<HandlerResponse> {
-    const handler = this.errorHandlerRegistry.resolve(error);
-    if (handler !== null) {
+    const visitedHandlers = new Set<ErrorHandler>();
+    let currentError = error;
+
+    while (true) {
+      const handler = this.errorHandlerRegistry.resolve(currentError);
+      if (handler === null || visitedHandlers.has(handler)) break;
+      visitedHandlers.add(handler);
+
       try {
         const { scope, ...reqCtx } = options;
-        const body = await handler.apply(scope ?? this, [error, reqCtx]);
+        const body = await handler.apply(scope ?? this, [currentError, reqCtx]);
         if (
           body instanceof Response ||
           isExtendedAPIGatewayProxyResult(body) ||
           isBinaryResult(body)
         ) {
           return body;
         }
-        return this.#errorBodyToWebResponse(body, error);
+        return this.#errorBodyToWebResponse(body, currentError);
       } catch (handlerError) {
-        if (handlerError instanceof HttpError) {
-          return await this.handleError(handlerError, options);
+        if (!(handlerError instanceof HttpError)) {
+          return this.#defaultErrorHandler(handlerError as Error);
         }
-        return this.#defaultErrorHandler(handlerError as Error);
+        currentError = handlerError;
       }
     }
 
-    if (error instanceof HttpError) {
-      return error.toWebResponse();
+    if (currentError instanceof HttpError) {
+      return currentError.toWebResponse();
     }
 
-    return this.#defaultErrorHandler(error);
+    return this.#defaultErrorHandler(currentError);
   }
 
   /**

packages/event-handler/tests/unit/http/Router/error-handling.test.ts

@@ -8,7 +8,185 @@ import {
   NotFoundError,
   Router,
 } from '../../../../src/http/index.js';
-import { createTestEvent, createTestEventV2 } from '../helpers.js';
+import {
+  createTestALBEvent,
+  createTestEvent,
+  createTestEventV2,
+} from '../helpers.js';
+
+describe.each([
+  { version: 'V1', createEvent: createTestEvent },
+  { version: 'V2', createEvent: createTestEventV2 },
+  { version: 'ALB', createEvent: createTestALBEvent },
+])('Class: Router - Error Handler Cycles ($version)', ({ createEvent }) => {
+  it.each([
+    { name: 'specific', errorClass: BadRequestError },
+    { name: 'catch-all', errorClass: Error },
+  ])(
+    'stops an async $name handler from re-entering',
+    async ({ errorClass }) => {
+      // Prepare
+      const app = new Router();
+      let calls = 0;
+      const errorHandler = vi.fn(async (error: Error) => {
+        calls += 1;
+        // Keep a regression from hanging the test runner.
+        if (calls > 50) throw new Error('Test bailout');
+        throw error;
+      });
+      app.errorHandler(errorClass, errorHandler);
+      app.get('/test', () => {
+        throw new BadRequestError('Invalid request');
+      });
+
+      // Act
+      const result = await app.resolve(createEvent('/test', 'GET'), context);
+
+      // Assess
+      expect(errorHandler).toHaveBeenCalledTimes(1);
+      expect(result.statusCode).toBe(400);
+      expect(JSON.parse(result.body ?? '{}')).toEqual({
+        statusCode: 400,
+        error: 'BadRequestError',
+        message: 'Invalid request',
+      });
+    }
+  );
+
+  it('stops a synchronous handler from throwing fresh matching errors repeatedly', async () => {
+    // Prepare
+    const app = new Router();
+    let calls = 0;
+    const errorHandler = vi.fn(() => {
+      calls += 1;
+      if (calls > 50) throw new Error('Test bailout');
+      throw new BadRequestError('Replacement error');
+    });
+    app.errorHandler(BadRequestError, errorHandler);
+    app.get('/test', () => {
+      throw new BadRequestError('Original error');
+    });
+
+    // Act
+    const result = await app.resolve(createEvent('/test', 'GET'), context);
+
+    // Assess
+    expect(errorHandler).toHaveBeenCalledTimes(1);
+    expect(result.statusCode).toBe(400);
+    expect(JSON.parse(result.body ?? '{}').message).toBe('Replacement error');
+  });
+
+  it('stops a cycle between two handlers', async () => {
+    // Prepare
+    const app = new Router();
+    let calls = 0;
+    const firstHandler = vi.fn(async () => {
+      calls += 1;
+      if (calls > 50) throw new Error('Test bailout');
+      throw new NotFoundError('Translated error');
+    });
+    const secondHandler = vi.fn(async () => {
+      throw new BadRequestError('Cycle returns to the first handler');
+    });
+    app.errorHandler(BadRequestError, firstHandler);
+    app.errorHandler(NotFoundError, secondHandler);
+    app.get('/test', () => {
+      throw new BadRequestError('Original error');
+    });
+
+    // Act
+    const result = await app.resolve(createEvent('/test', 'GET'), context);
+
+    // Assess
+    expect(firstHandler).toHaveBeenCalledTimes(1);
+    expect(secondHandler).toHaveBeenCalledTimes(1);
+    expect(result.statusCode).toBe(400);
+    expect(JSON.parse(result.body ?? '{}').message).toBe(
+      'Cycle returns to the first handler'
+    );
+  });
+
+  it('tracks a shared handler registered for multiple error types', async () => {
+    // Prepare
+    const app = new Router();
+    let calls = 0;
+    const errorHandler = vi.fn(async () => {
+      calls += 1;
+      if (calls > 50) throw new Error('Test bailout');
+      throw new NotFoundError('Translated error');
+    });
+    app.errorHandler<Error>([BadRequestError, NotFoundError], errorHandler);
+    app.get('/test', () => {
+      throw new BadRequestError('Original error');
+    });
+
+    // Act
+    const result = await app.resolve(createEvent('/test', 'GET'), context);
+
+    // Assess
+    expect(errorHandler).toHaveBeenCalledTimes(1);
+    expect(result.statusCode).toBe(404);
+    expect(JSON.parse(result.body ?? '{}').message).toBe('Translated error');
+  });
+
+  it('allows delegation to a different error handler', async () => {
+    // Prepare
+    const app = new Router();
+    const firstHandler = vi.fn(async () => {
+      throw new NotFoundError('Translated error');
+    });
+    const secondHandler = vi.fn(
+      async () => new Response('Custom not found', { status: 404 })
+    );
+    app.errorHandler(BadRequestError, firstHandler);
+    app.errorHandler(NotFoundError, secondHandler);
+    app.get('/test', () => {
+      throw new BadRequestError('Original error');
+    });
+
+    // Act
+    const result = await app.resolve(createEvent('/test', 'GET'), context);
+
+    // Assess
+    expect(firstHandler).toHaveBeenCalledTimes(1);
+    expect(secondHandler).toHaveBeenCalledTimes(1);
+    expect(result.statusCode).toBe(404);
+    expect(result.body).toBe('Custom not found');
+  });
+
+  it('keeps error handler tracking local to concurrent requests', async () => {
+    // Prepare
+    const app = new Router();
+    const entered = Promise.withResolvers<void>();
+    const release = Promise.withResolvers<void>();
+    let calls = 0;
+    const errorHandler = vi.fn(async (error: Error) => {
+      calls += 1;
+      if (calls > 50) throw new Error('Test bailout');
+      entered.resolve();
+      await release.promise;
+      throw error;
+    });
+    app.errorHandler(BadRequestError, errorHandler);
+    app.get('/test/:id', ({ req }) => {
+      throw new BadRequestError(new URL(req.url).pathname);
+    });
+
+    // Act
+    const first = app.resolve(createEvent('/test/first', 'GET'), context);
+    await entered.promise;
+    const second = app.resolve(createEvent('/test/second', 'GET'), context);
+    release.resolve();
+    const results = await Promise.all([first, second]);
+
+    // Assess
+    expect(errorHandler).toHaveBeenCalledTimes(2);
+    expect(results.map((result) => result.statusCode)).toEqual([400, 400]);
+    expect(
+      results.map((result) => JSON.parse(result.body ?? '{}').message)
+    ).toEqual(['/test/first', '/test/second']);
+  });
+});
 
 describe.each([
   { version: 'V1', createEvent: createTestEvent },

packages/event-handler/tests/unit/http/Router/streaming.test.ts

@@ -2,6 +2,7 @@ import { Duplex, PassThrough, Readable } from 'node:stream';
 import context from '@aws-lambda-powertools/testing-utils/context';
 import { describe, expect, it, vi } from 'vitest';
 import {
+  BadRequestError,
   Router,
   streamify,
   UnauthorizedError,
@@ -16,6 +17,35 @@ describe.each([
   { version: 'V1', createEvent: createTestEvent },
   { version: 'V2', createEvent: createTestEventV2 },
 ])('Class: Router - Streaming ($version)', ({ createEvent }) => {
+  it('streams the built-in HTTP response when an error handler rethrows', async () => {
+    // Prepare
+    const app = new Router();
+    let calls = 0;
+    const errorHandler = vi.fn(async (error: Error) => {
+      calls += 1;
+      if (calls > 50) throw new Error('Test bailout');
+      throw error;
+    });
+    app.errorHandler(BadRequestError, errorHandler);
+    app.get('/test', () => {
+      throw new BadRequestError('Invalid request');
+    });
+    const handler = streamify(app);
+    const responseStream = new ResponseStream();
+
+    // Act
+    const result = await handler(
+      createEvent('/test', 'GET'),
+      responseStream,
+      context
+    );
+
+    // Assess
+    expect(errorHandler).toHaveBeenCalledTimes(1);
+    expect(result.statusCode).toBe(400);
+    expect(JSON.parse(result.body).message).toBe('Invalid request');
+  });
+
   it('streams a simple JSON response', async () => {
     // Prepare
     const app = new Router();