From 8a9b7cf8581427967b42ada809007617dad6e957 Mon Sep 17 00:00:00 2001 From: Marc MacLeod Date: Mon, 29 Jan 2024 15:09:01 -0600 Subject: [PATCH] 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> --- src/listener.ts | 39 +++++++++++++++---- src/types.ts | 2 + test/listener.test.ts | 91 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 125 insertions(+), 7 deletions(-) create mode 100644 test/listener.test.ts diff --git a/src/listener.ts b/src/listener.ts index bd70584..2388344 100644 --- 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, - 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 index 40e898c..3dfb2f4 100644 --- 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 diff --git a/test/listener.test.ts b/test/listener.test.ts new file mode 100644 index 0000000..5c10d84 --- /dev/null +++ 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') + }) +})