feat(listener): support custom error handler (#131)

* feat(listener): support custom error handler Signed-off-by: Marc MacLeod <847542+marbemac@users.noreply.github.com> * pr feedback Signed-off-by: Marc MacLeod <847542+marbemac@users.noreply.github.com> * add tests for custom error handling Signed-off-by: Marc MacLeod <847542+marbemac@users.noreply.github.com> * format Signed-off-by: Marc MacLeod <847542+marbemac@users.noreply.github.com> * lint fix Signed-off-by: Marc MacLeod <847542+marbemac@users.noreply.github.com> --------- Signed-off-by: Marc MacLeod <847542+marbemac@users.noreply.github.com>
honojs · Jan 29, 2024 · 8a9b7cf · 8a9b7cf
1 parent 3190f81
commit 8a9b7cf
Show file tree

Hide file tree

Showing 3 changed files with 125 additions and 7 deletions.
diff --git a/src/listener.ts b/src/listener.ts
@@ -2,7 +2,7 @@ import type { IncomingMessage, ServerResponse, OutgoingHttpHeaders } from 'node:
 import type { Http2ServerRequest, Http2ServerResponse } from 'node:http2'
 import { newRequest } from './request'
 import { cacheKey } from './response'
-import type { FetchCallback, HttpBindings } from './types'
+import type { CustomErrorHandler, FetchCallback, HttpBindings } from './types'
 import { writeFromReadableStream, buildOutgoingHttpHeaders } from './utils'
 import './globals'
 
@@ -53,9 +53,24 @@ const responseViaCache = (
 
 const responseViaResponseObject = async (
   res: Response | Promise<Response>,
-  outgoing: ServerResponse | Http2ServerResponse
+  outgoing: ServerResponse | Http2ServerResponse,
+  options: { errorHandler?: CustomErrorHandler } = {}
 ) => {
-  res = res instanceof Promise ? await res.catch(handleFetchError) : res
+  if (res instanceof Promise) {
+    if (options.errorHandler) {
+      try {
+        res = await res
+      } catch (err) {
+        const errRes = await options.errorHandler(err)
+        if (!errRes) {
+          return
+        }
+        res = errRes
+      }
+    } else {
+      res = await res.catch(handleFetchError)
+    }
+  }
 
   try {
     const isCached = cacheKey in res
@@ -114,8 +129,11 @@ const responseViaResponseObject = async (
   }
 }
 
-export const getRequestListener = (fetchCallback: FetchCallback) => {
-  return (
+export const getRequestListener = (
+  fetchCallback: FetchCallback,
+  options: { errorHandler?: CustomErrorHandler } = {}
+) => {
+  return async (
     incoming: IncomingMessage | Http2ServerRequest,
     outgoing: ServerResponse | Http2ServerResponse
   ) => {
@@ -135,12 +153,19 @@ export const getRequestListener = (fetchCallback: FetchCallback) => {
       }
     } catch (e: unknown) {
       if (!res) {
-        res = handleFetchError(e)
+        if (options.errorHandler) {
+          res = await options.errorHandler(e)
+          if (!res) {
+            return
+          }
+        } else {
+          res = handleFetchError(e)
+        }
       } else {
         return handleResponseError(e, outgoing)
       }
     }
 
-    return responseViaResponseObject(res, outgoing)
+    return responseViaResponseObject(res, outgoing, options)
   }
 }
diff --git a/src/types.ts b/src/types.ts
@@ -72,3 +72,5 @@ export type Options = {
   port?: number
   hostname?: string
 } & ServerOptions
+
+export type CustomErrorHandler = (err: unknown) => void | Response | Promise<void | Response>
diff --git a/test/listener.test.ts b/test/listener.test.ts
@@ -0,0 +1,91 @@
+import { createServer } from 'node:http'
+import request from 'supertest'
+import { getRequestListener } from '../src/listener'
+
+describe('Error handling - sync fetchCallback', () => {
+  const fetchCallback = jest.fn(() => {
+    throw new Error('thrown error')
+  })
+  const errorHandler = jest.fn()
+
+  const requestListener = getRequestListener(fetchCallback, { errorHandler })
+
+  const server = createServer(async (req, res) => {
+    await requestListener(req, res)
+
+    if (!res.writableEnded) {
+      res.writeHead(500, { 'Content-Type': 'text/plain' })
+      res.end('error handler did not return a response')
+    }
+  })
+
+  beforeEach(() => {
+    errorHandler.mockReset()
+  })
+
+  it('Should set the response if error handler returns a response', async () => {
+    errorHandler.mockImplementationOnce((err: Error) => {
+      return new Response(`${err}`, { status: 500, headers: { 'my-custom-header': 'hi' } })
+    })
+
+    const res = await request(server).get('/throw-error')
+    expect(res.status).toBe(500)
+    expect(res.headers['my-custom-header']).toBe('hi')
+    expect(res.text).toBe('Error: thrown error')
+  })
+
+  it('Should not set the response if the error handler does not return a response', async () => {
+    errorHandler.mockImplementationOnce(() => {
+      // do something else, such as passing error to vite next middleware, etc
+    })
+
+    const res = await request(server).get('/throw-error')
+    expect(errorHandler).toHaveBeenCalledTimes(1)
+    expect(res.status).toBe(500)
+    expect(res.text).toBe('error handler did not return a response')
+  })
+})
+
+describe('Error handling - async fetchCallback', () => {
+  const fetchCallback = jest.fn(async () => {
+    throw new Error('thrown error')
+  })
+  const errorHandler = jest.fn()
+
+  const requestListener = getRequestListener(fetchCallback, { errorHandler })
+
+  const server = createServer(async (req, res) => {
+    await requestListener(req, res)
+
+    if (!res.writableEnded) {
+      res.writeHead(500, { 'Content-Type': 'text/plain' })
+      res.end('error handler did not return a response')
+    }
+  })
+
+  beforeEach(() => {
+    errorHandler.mockReset()
+  })
+
+  it('Should set the response if error handler returns a response', async () => {
+    errorHandler.mockImplementationOnce((err: Error) => {
+      return new Response(`${err}`, { status: 500, headers: { 'my-custom-header': 'hi' } })
+    })
+
+    const res = await request(server).get('/throw-error')
+    expect(res.status).toBe(500)
+    expect(res.headers['my-custom-header']).toBe('hi')
+    expect(res.text).toBe('Error: thrown error')
+  })
+
+  it('Should not set the response if the error handler does not return a response', async () => {
+    errorHandler.mockImplementationOnce(() => {
+      // do something else, such as passing error to vite next middleware, etc
+    })
+
+    const res = await request(server).get('/throw-error')
+    expect(errorHandler).toHaveBeenCalledTimes(1)
+    expect(res.status).toBe(500)
+    expect(res.text).toBe('error handler did not return a response')
+  })
+})