From f92af9b5c89079fe1418b7ed2ab3986314d11b7e Mon Sep 17 00:00:00 2001 From: TheoryOfNekomata Date: Wed, 17 Apr 2024 11:20:33 +0800 Subject: [PATCH] Streamline error handling Implement default error handler, unify error handling logic in server. --- .../core/src/backend/servers/http/core.ts | 93 ++++++++++++------- packages/core/src/common/resource.ts | 4 +- 2 files changed, 62 insertions(+), 35 deletions(-) diff --git a/packages/core/src/backend/servers/http/core.ts b/packages/core/src/backend/servers/http/core.ts index b7c506a..4b75281 100644 --- a/packages/core/src/backend/servers/http/core.ts +++ b/packages/core/src/backend/servers/http/core.ts @@ -160,8 +160,11 @@ class CqrsEventEmitter extends EventEmitter { } +export type ErrorHandler = (err?: E) => never; + interface ServerState { requestDecorators: Set; + errorHandler: ErrorHandler; } export interface Server { @@ -170,12 +173,17 @@ export interface Server { close(callback?: (err?: Error) => void): this; listen(...args: Parameters): this; requestDecorator(requestDecorator: RequestDecorator): this; - defaultErrorHandler(): this; + defaultErrorHandler(errorHandler: ErrorHandler): this; } +const DEFAULT_ERROR_HANDLER: ErrorHandler = (err) => { + throw err; +}; + export const createServer = (backendState: BackendState, serverParams = {} as CreateServerParams) => { const state: ServerState = { requestDecorators: new Set(), + errorHandler: DEFAULT_ERROR_HANDLER, }; const isHttps = 'key' in serverParams && 'cert' in serverParams; @@ -391,7 +399,7 @@ export const createServer = (backendState: BackendState, serverParams = {} as Cr ); }; - const handleMiddlewareError = (processRequestErrRaw: Error) => (resourceReq: ResourceRequestContext, res: http.ServerResponse) => { + const handleResourceError = (processRequestErrRaw: Error) => (resourceReq: ResourceRequestContext, res: http.ServerResponse) => { const finalErr = processRequestErrRaw as ErrorPlainResponse; const headers = finalErr.headers ?? {}; const language = resourceReq.cn.language ?? resourceReq.backend.cn.language; @@ -439,6 +447,12 @@ export const createServer = (backendState: BackendState, serverParams = {} as Cr res.end(); }; + const handleError = (err: Error) => (req: RequestContext, res: http.ServerResponse) => { + if ('resource' in req && typeof req.resource !== 'undefined') { + handleResourceError(err)(req as ResourceRequestContext, res); + } + }; + const handleRequest = async (reqRaw: RequestContext, res: http.ServerResponse) => { const plainReq = await decorateRequest(reqRaw); // TODO add type safety here, put handleGetRoot as its own middleware as it does not concern over any resource const language = plainReq.cn.language ?? plainReq.backend.cn.language; @@ -462,7 +476,7 @@ export const createServer = (backendState: BackendState, serverParams = {} as Cr middlewareState = await processRequestFn(resourceReq) as any; // TODO fix this } catch (processRequestErrRaw) { // TODO add error handlers - handleMiddlewareError(processRequestErrRaw as Error)(resourceReq, res); + handleError(processRequestErrRaw as Error)(resourceReq, res); return; } @@ -501,23 +515,25 @@ export const createServer = (backendState: BackendState, serverParams = {} as Cr return `${mimeType}/merge-patch+${mimeSubtype}`; }).join(','); } - res.statusMessage = language.statusMessages['unableToSerializeResponse']?.replace( - /\$RESOURCE/g, - resourceReq.resource!.state.itemName) ?? ''; - res.writeHead(constants.HTTP_STATUS_INTERNAL_SERVER_ERROR, headers); - res.end(); + + handleError(new ErrorPlainResponse('unableToSerializeResponse', { + cause, + statusCode: constants.HTTP_STATUS_INTERNAL_SERVER_ERROR, + headers, + res, + }))(resourceReq, res); return; } try { encoded = charset.encode(serialized); } catch (cause) { - res.statusMessage = language.statusMessages['unableToEncodeResponse']?.replace(/\$RESOURCE/g, - resourceReq.resource!.state.itemName) ?? ''; - res.writeHead(constants.HTTP_STATUS_INTERNAL_SERVER_ERROR, { - 'Content-Language': language.name, - }); - res.end(); + handleError(new ErrorPlainResponse('unableToEncodeResponse', { + cause, + statusCode: constants.HTTP_STATUS_INTERNAL_SERVER_ERROR, + headers, + res, + }))(resourceReq, res); return; } @@ -539,30 +555,38 @@ export const createServer = (backendState: BackendState, serverParams = {} as Cr } if (middlewares.length > 0) { - res.statusMessage = language.statusMessages.methodNotAllowed.replace(/\$RESOURCE/g, - resourceReq.resource!.state.itemName) ?? ''; - res.writeHead(constants.HTTP_STATUS_METHOD_NOT_ALLOWED, { - Allow: middlewares.map((m) => m.method).join(', '), - 'Content-Language': language.name, - }); - res.end(); + handleError(new ErrorPlainResponse('methodNotAllowed', { + statusCode: constants.HTTP_STATUS_METHOD_NOT_ALLOWED, + res, + headers: { + Allow: middlewares.map((m) => m.method).join(', '), + }, + }))(resourceReq, res); return; } - res.statusMessage = language.statusMessages.urlNotFound.replace(/\$RESOURCE/g, - resourceReq.resource!.state.itemName) ?? ''; - res.writeHead(constants.HTTP_STATUS_NOT_FOUND, { - 'Content-Language': language.name, - }); - res.end(); + + handleError(new ErrorPlainResponse('urlNotFound', { + statusCode: constants.HTTP_STATUS_NOT_FOUND, + res, + }))(resourceReq, res); return; } - res.statusMessage = language.statusMessages.notImplemented.replace(/\$RESOURCE/g, - reqRaw.resource!.state.itemName) ?? ''; - res.writeHead(constants.HTTP_STATUS_NOT_IMPLEMENTED, { - 'Content-Language': language.name, - }); - res.end(); + try { + state.errorHandler(); + } catch (err) { + handleError(err as Error)(reqRaw, res); + //handleMiddlewareError(err)(reqRaw, res); + return; + } + // TODO default error handling + + handleError( + new ErrorPlainResponse('urlNotFound', { + statusCode: constants.HTTP_STATUS_NOT_FOUND, + res, + }) + )(reqRaw, res); }; server.on('request', handleRequest); @@ -585,7 +609,8 @@ export const createServer = (backendState: BackendState, serverParams = {} as Cr state.requestDecorators.add(requestDecorator); return this; }, - defaultErrorHandler() { + defaultErrorHandler(errorHandler: ErrorHandler) { + state.errorHandler = errorHandler; return this; } } satisfies Server; diff --git a/packages/core/src/common/resource.ts b/packages/core/src/common/resource.ts index e5b4fe5..10a3a37 100644 --- a/packages/core/src/common/resource.ts +++ b/packages/core/src/common/resource.ts @@ -79,7 +79,9 @@ export const resource = { - return Object.freeze(resourceState); + return Object.freeze({ + ...resourceState, + }); }, canFetchCollection(b = true) { resourceState.canFetchCollection = b;