From 0c1045fdc9f2d1fbc402e68a7deb5c7393c1f4a9 Mon Sep 17 00:00:00 2001
From: Adam Jones <domdomegg+git@gmail.com>
Date: Wed, 20 Dec 2023 05:13:34 +0000
Subject: [PATCH] fix(#5220): better support encapsulated synchronous error
 handlers

Parent error handling for synchronous error handlers was largely a unintended byproduct of some other try catches previously.

This could result in a server crash if multiple error handlers were used, and rethrew errors (see #5220).

This PR fixes this behavior by retrying synchronous error handling (similar to how we already do in wrapThenable for asynchronous error handling). It also adds regression tests to ensure this continues to work properly.
---
 lib/error-handler.js                    | 20 ++++--
 test/encapsulated-error-handler.test.js | 91 +++++++++++++++++++++++++
 2 files changed, 104 insertions(+), 7 deletions(-)

diff --git a/lib/error-handler.js b/lib/error-handler.js
index 32cee75107..954c2bc74d 100644
--- a/lib/error-handler.js
+++ b/lib/error-handler.js
@@ -7,7 +7,8 @@ const {
   kReplyNextErrorHandler,
   kReplyIsRunningOnErrorHook,
   kReplyHasStatusCode,
-  kRouteContext
+  kRouteContext,
+  kReplyIsError
 } = require('./symbols.js')
 
 const {
@@ -62,13 +63,18 @@ function handleError (reply, error, cb) {
     return
   }
 
-  const result = func(error, reply.request, reply)
-  if (result !== undefined) {
-    if (result !== null && typeof result.then === 'function') {
-      wrapThenable(result, reply)
-    } else {
-      reply.send(result)
+  try {
+    const result = func(error, reply.request, reply)
+    if (result !== undefined) {
+      if (result !== null && typeof result.then === 'function') {
+        wrapThenable(result, reply)
+      } else {
+        reply.send(result)
+      }
     }
+  } catch (err) {
+    reply[kReplyIsError] = true
+    reply.send(err)
   }
 }
 
diff --git a/test/encapsulated-error-handler.test.js b/test/encapsulated-error-handler.test.js
index aba8c1a8c1..0d2501f17c 100644
--- a/test/encapsulated-error-handler.test.js
+++ b/test/encapsulated-error-handler.test.js
@@ -48,3 +48,94 @@ test('onError hook nested', async t => {
   const res = await fastify.inject('/encapsulated')
   t.equal(res.json().message, 'wrapped')
 })
+
+// See https://github.com/fastify/fastify/issues/5220
+test('encapuslates an error handler, for errors thrown in hooks', async t => {
+  t.plan(3)
+
+  const fastify = Fastify()
+  fastify.register(async function (fastify) {
+    fastify.setErrorHandler(function a (err) {
+      t.equal(err.message, 'from_hook')
+      throw new Error('from_inner')
+    })
+    fastify.addHook('onRequest', async () => { throw new Error('from_hook') })
+    fastify.get('/encapsulated', async () => {})
+  })
+
+  fastify.setErrorHandler(function b (err) {
+    t.equal(err.message, 'from_inner')
+    throw new Error('from_outer')
+  })
+
+  const res = await fastify.inject('/encapsulated')
+  t.equal(res.json().message, 'from_outer')
+})
+
+// See https://github.com/fastify/fastify/issues/5220
+test('encapuslates many synchronous error handlers that rethrow errors', { only: true }, async t => {
+  const DEPTH = 100
+  t.plan(DEPTH + 2)
+
+  const createNestedRoutes = (fastify, depth) => {
+    if (depth < 0) {
+      throw new Error('Expected depth >= 0')
+    } else if (depth === 0) {
+      fastify.setErrorHandler(function a (err) {
+        t.equal(err.message, 'from_route')
+        throw new Error(`from_handler_${depth}`)
+      })
+      fastify.get('/encapsulated', async () => { throw new Error('from_route') })
+    } else {
+      fastify.setErrorHandler(function d (err) {
+        t.equal(err.message, `from_handler_${depth - 1}`)
+        throw new Error(`from_handler_${depth}`)
+      })
+
+      fastify.register(async function (fastify) {
+        createNestedRoutes(fastify, depth - 1)
+      })
+    }
+  }
+
+  const fastify = Fastify()
+  createNestedRoutes(fastify, DEPTH)
+
+  const res = await fastify.inject('/encapsulated')
+  t.equal(res.json().message, `from_handler_${DEPTH}`)
+})
+
+// See https://github.com/fastify/fastify/issues/5220
+// This was not failing previously, but we want to make sure the behavior continues to work in the same way across async and sync handlers
+// Plus, the current setup is somewhat fragile to tweaks to wrapThenable as that's what retries (by calling res.send(err) again)
+test('encapuslates many asynchronous error handlers that rethrow errors', { only: true }, async t => {
+  const DEPTH = 100
+  t.plan(DEPTH + 2)
+
+  const createNestedRoutes = (fastify, depth) => {
+    if (depth < 0) {
+      throw new Error('Expected depth >= 0')
+    } else if (depth === 0) {
+      fastify.setErrorHandler(async function a (err) {
+        t.equal(err.message, 'from_route')
+        throw new Error(`from_handler_${depth}`)
+      })
+      fastify.get('/encapsulated', async () => { throw new Error('from_route') })
+    } else {
+      fastify.setErrorHandler(async function m (err) {
+        t.equal(err.message, `from_handler_${depth - 1}`)
+        throw new Error(`from_handler_${depth}`)
+      })
+
+      fastify.register(async function (fastify) {
+        createNestedRoutes(fastify, depth - 1)
+      })
+    }
+  }
+
+  const fastify = Fastify()
+  createNestedRoutes(fastify, DEPTH)
+
+  const res = await fastify.inject('/encapsulated')
+  t.equal(res.json().message, `from_handler_${DEPTH}`)
+})