fix: run middleware upstream flow when route middleware or handler throws an error

Author: thetutlage

src/router/executor.ts

@@ -10,15 +10,22 @@
 import type { ContainerResolver } from '@adonisjs/fold'
 import type { StoreRouteNode } from '../types/route.js'
 import type { HttpContext } from '../http_context/main.js'
+import type { ServerErrorHandler } from '../types/server.js'
 import { useReturnValue } from './factories/use_return_value.js'
 
 /**
  * Executor to execute the route middleware pipeline the route
  * handler
  */
-export function execute(route: StoreRouteNode, resolver: ContainerResolver<any>, ctx: HttpContext) {
+export function execute(
+  route: StoreRouteNode,
+  resolver: ContainerResolver<any>,
+  ctx: HttpContext,
+  errorResponder: ServerErrorHandler['handle']
+) {
   return route.middleware
     .runner()
+    .errorHandler((error) => errorResponder(error, ctx))
     .finalHandler(async () => {
       if (typeof route.handler === 'function') {
         return Promise.resolve(route.handler(ctx)).then(useReturnValue(ctx))

src/server/factories/final_handler.ts

@@ -12,13 +12,19 @@ import type { ContainerResolver } from '@adonisjs/fold'
 import * as errors from '../../exceptions.js'
 import type { Router } from '../../router/main.js'
 import type { HttpContext } from '../../http_context/main.js'
+import type { ServerErrorHandler } from '../../types/server.js'
 
 /**
  * The final handler is executed after the server middleware stack.
  * It looks for a matching route and executes the route middleware
  * stack.
  */
-export function finalHandler(router: Router, resolver: ContainerResolver<any>, ctx: HttpContext) {
+export function finalHandler(
+  router: Router,
+  resolver: ContainerResolver<any>,
+  ctx: HttpContext,
+  errorResponder: ServerErrorHandler['handle']
+) {
   return function () {
     const url = ctx.request.url()
     const method = ctx.request.method()
@@ -30,7 +36,7 @@ export function finalHandler(router: Router, resolver: ContainerResolver<any>, c
       ctx.subdomains = route.subdomains
       ctx.route = route.route
       ctx.routeKey = route.routeKey
-      return route.route.execute(route.route, resolver, ctx)
+      return route.route.execute(route.route, resolver, ctx, errorResponder)
     }
 
     return Promise.reject(new errors.E_ROUTE_NOT_FOUND([method, url]))

src/server/main.ts

@@ -115,6 +115,19 @@ export class Server {
    */
   #middleware: ParsedGlobalMiddleware[] = []
 
+  /**
+   * The request error response is attached to the middleware
+   * pipeline to intercept errors and invoke the user
+   * registered error handler.
+   *
+   * We share this with the route middleware pipeline as well,
+   * so that it does not throw any exceptions
+   */
+  #requestErrorResponder: ServerErrorHandler['handle'] = (error, ctx) => {
+    this.#resolvedErrorHandler.report(error, ctx)
+    return this.#resolvedErrorHandler.handle(error, ctx)
+  }
+
   /**
    * Know if async local storage is enabled or not.
    */
@@ -168,11 +181,8 @@ export class Server {
    */
   #handleRequest(ctx: HttpContext, resolver: ContainerResolver<any>) {
     return this.#serverMiddlewareStack!.runner()
-      .errorHandler((error) => {
-        this.#resolvedErrorHandler.report(error, ctx)
-        return this.#resolvedErrorHandler.handle(error, ctx)
-      })
-      .finalHandler(finalHandler(this.#router!, resolver, ctx))
+      .errorHandler((error) => this.#requestErrorResponder(error, ctx))
+      .finalHandler(finalHandler(this.#router!, resolver, ctx, this.#requestErrorResponder))
       .run(middlewareHandler(resolver, ctx))
       .catch((error) => {
         ctx.logger.fatal({ err: error }, 'Exception raised by error handler')

src/types/route.ts

@@ -13,6 +13,7 @@ import type { ContainerResolver } from '@adonisjs/fold'
 import type { Constructor, LazyImport } from './base.js'
 import type { HttpContext } from '../http_context/main.js'
 import type { MiddlewareFn, ParsedGlobalMiddleware } from './middleware.js'
+import { ServerErrorHandler } from './server.js'
 
 /**
  * Returns a union of methods from a controller that accepts
@@ -70,7 +71,12 @@ export type StoreRouteNode = {
    * The execute function to execute the route middleware
    * and the handler
    */
-  execute: (route: StoreRouteNode, resolver: ContainerResolver<any>, ctx: HttpContext) => any
+  execute: (
+    route: StoreRouteNode,
+    resolver: ContainerResolver<any>,
+    ctx: HttpContext,
+    errorResponder: ServerErrorHandler['handle']
+  ) => any
 
   /**
    * A unique name for the route

tests/router/execute.spec.ts

@@ -42,7 +42,7 @@ test.group('Route | execute', () => {
     })
 
     const routeJSON = route.toJSON()
-    await routeJSON.execute(routeJSON, app.container.createResolver(), context)
+    await routeJSON.execute(routeJSON, app.container.createResolver(), context, () => {})
     assert.deepEqual(stack, ['handler'])
   })
 
@@ -75,7 +75,7 @@ test.group('Route | execute', () => {
       },
     }
 
-    await routeJSON.execute(routeJSON, resolver, context)
+    await routeJSON.execute(routeJSON, resolver, context, () => {})
     assert.deepEqual(stack, ['controller'])
   })
 
@@ -111,7 +111,7 @@ test.group('Route | execute', () => {
     })
 
     const routeJSON = route.toJSON()
-    await routeJSON.execute(routeJSON, resolver, context)
+    await routeJSON.execute(routeJSON, resolver, context, () => {})
 
     const route1 = new Route(app, [], {
       pattern: '/',
@@ -121,7 +121,7 @@ test.group('Route | execute', () => {
     })
 
     const route1JSON = route1.toJSON()
-    await route1JSON.execute(route1JSON, resolver, context)
+    await route1JSON.execute(route1JSON, resolver, context, () => {})
 
     assert.deepEqual(stack, ['invoked', 'invoked handle'])
   })
@@ -152,7 +152,7 @@ test.group('Route | execute', () => {
     })
 
     const routeJSON = route.toJSON()
-    await routeJSON.execute(routeJSON, resolver, context)
+    await routeJSON.execute(routeJSON, resolver, context, () => {})
 
     const route1 = new Route(app, [], {
       pattern: '/',
@@ -162,7 +162,7 @@ test.group('Route | execute', () => {
     })
 
     const route1JSON = route1.toJSON()
-    await route1JSON.execute(route1JSON, resolver, context)
+    await route1JSON.execute(route1JSON, resolver, context, () => {})
 
     assert.deepEqual(stack, ['invoked', 'invoked handle'])
   })
@@ -200,7 +200,7 @@ test.group('Route | execute', () => {
     })
 
     const routeJSON = route.toJSON()
-    await routeJSON.execute(routeJSON, app.container.createResolver(), context)
+    await routeJSON.execute(routeJSON, app.container.createResolver(), context, () => {})
     assert.deepEqual(stack, ['middleware 1', 'middleware 2', 'handler'])
   })
 
@@ -238,7 +238,7 @@ test.group('Route | execute', () => {
     })
 
     const routeJSON = route.toJSON()
-    await routeJSON.execute(routeJSON, app.container.createResolver(), context)
+    await routeJSON.execute(routeJSON, app.container.createResolver(), context, () => {})
     assert.deepEqual(stack, ['middleware 1', 'middleware 2'])
   })
 
@@ -303,7 +303,7 @@ test.group('Route | execute', () => {
     })
 
     const routeJSON = route.toJSON()
-    await routeJSON.execute(routeJSON, app.container.createResolver(), context)
+    await routeJSON.execute(routeJSON, app.container.createResolver(), context, () => {})
     assert.deepEqual(stack, ['bodyparser', 'log', 'middleware 1', 'middleware 2', 'handler'])
   })
 
@@ -369,7 +369,7 @@ test.group('Route | execute', () => {
     })
 
     const routeJSON = route.toJSON()
-    await routeJSON.execute(routeJSON, app.container.createResolver(), context)
+    await routeJSON.execute(routeJSON, app.container.createResolver(), context, () => {})
     assert.deepEqual(stack, ['bodyparser', 'log'])
   })
 
@@ -430,10 +430,9 @@ test.group('Route | execute', () => {
     })
 
     const routeJSON = route.toJSON()
-    await assert.rejects(
-      () => routeJSON.execute(routeJSON, app.container.createResolver(), context),
-      'Log middleware failed'
-    )
+    await routeJSON.execute(routeJSON, app.container.createResolver(), context, (error) => {
+      assert.equal(error.message, 'Log middleware failed')
+    })
 
     assert.deepEqual(stack, ['bodyparser', 'log'])
   })
@@ -494,10 +493,9 @@ test.group('Route | execute', () => {
     })
 
     const routeJSON = route.toJSON()
-    await assert.rejects(
-      () => routeJSON.execute(routeJSON, app.container.createResolver(), context),
-      'route handler failed'
-    )
+    await routeJSON.execute(routeJSON, app.container.createResolver(), context, (error) => {
+      assert.equal(error.message, 'route handler failed')
+    })
 
     assert.deepEqual(stack, ['bodyparser', 'log', 'middleware 1', 'middleware 2'])
   })
@@ -546,11 +544,11 @@ test.group('Route | execute', () => {
     route1.middleware(namedMiddleware.acl({ role: 'admin' }))
 
     const routeJSON = route.toJSON()
-    await routeJSON.execute(routeJSON, app.container.createResolver(), context)
+    await routeJSON.execute(routeJSON, app.container.createResolver(), context, () => {})
     assert.deepEqual(stack, [undefined, 'handler'])
 
     const route1JSON = route1.toJSON()
-    await route1JSON.execute(route1JSON, app.container.createResolver(), context)
+    await route1JSON.execute(route1JSON, app.container.createResolver(), context, () => {})
     assert.deepEqual(stack, [undefined, 'handler', { role: 'admin' }, 'handler'])
   })
 })