From b94dab24d9136b4886115553b0594b783bce7826 Mon Sep 17 00:00:00 2001 From: saltyaom Date: Fri, 27 Dec 2024 19:15:13 +0700 Subject: [PATCH] :wrench: fix: handle error thrown as error --- CHANGELOG.md | 3 ++ example/a.ts | 22 ++++++---- src/adapter/types.ts | 6 +++ src/adapter/web-standard/handler.ts | 34 ++++++++++++--- src/compose.ts | 34 ++++++++------- src/dynamic-handle.ts | 9 ++-- src/types.ts | 34 ++++++++++----- test/core/handle-error.test.ts | 64 +++++++++++++++++++++++++++-- 8 files changed, 159 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 011301e5..fbbf4999 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,8 +2,11 @@ Bug fix: - macro doesn't work with guard - [#981](https://github.com/elysiajs/elysia/issues/981) unable to deference schema, create default, and coerce value +- [#966](https://github.com/elysiajs/elysia/issues/966) `error`'s value return as-if when thrown - [#964](https://github.com/elysiajs/elysia/issues/964) InvalidCookieSignature errors are not caught by onError - [#952](https://github.com/elysiajs/elysia/issues/952) onAfterResponse does not provide mapped response value unless aot is disabled +- `mapResponse.response` is `{}` if no response schema is provided +- Response doesn't reconcile when handler return `Response` is used with `mapResponse` # 1.2.6 - 25 Dec 2024 Bug fix: diff --git a/example/a.ts b/example/a.ts index 761d34be..efcf27e0 100644 --- a/example/a.ts +++ b/example/a.ts @@ -1,13 +1,17 @@ -import { Elysia, t } from '../src' +import { Elysia, t, error } from '../src' import { req } from '../test/utils' -const app = new Elysia({ aot: false }) - .onAfterResponse(({ response }) => { - console.log('Response:', response) +const res = await new Elysia({ + aot: false +}) + .get('/', () => 'Hi') + .onError(({ code }) => { + if (code === 'NOT_FOUND') + return new Response("I'm a teapot", { + status: 418 + }) }) - .get('/', async () => { - return { ok: true } - }) - .listen(3000) + .handle(req('/not-found')) -app.handle(req('/')) +// console.log(await res.text()) +// console.log(res.status) diff --git a/src/adapter/types.ts b/src/adapter/types.ts index 1a142691..e64eb45e 100644 --- a/src/adapter/types.ts +++ b/src/adapter/types.ts @@ -129,6 +129,12 @@ export interface ElysiaAdapter { inject?: Record mapResponseContext: string validationError: string + /** + * Handle thrown error which is instance of Error + * + * Despite its name of `unknownError`, it also handle named error like `NOT_FOUND`, `VALIDATION_ERROR` + * It's named `unknownError` because it also catch unknown error + */ unknownError: string } ws?(app: AnyElysia, path: string, handler: AnyWSLocalHook): unknown diff --git a/src/adapter/web-standard/handler.ts b/src/adapter/web-standard/handler.ts index 6453bce8..f596d41a 100644 --- a/src/adapter/web-standard/handler.ts +++ b/src/adapter/web-standard/handler.ts @@ -229,6 +229,17 @@ export const mergeResponseWithSetHeaders = ( response: Response, set: Context['set'] ) => { + if ( + (response as Response).status !== set.status && + set.status !== 200 && + ((response.status as number) <= 300 || + (response.status as number) > 400) + ) + response = new Response(response.body, { + headers: response.headers, + status: set.status as number + }) + let isCookieSet = false if (set.headers instanceof Headers) @@ -246,8 +257,7 @@ export const mergeResponseWithSetHeaders = ( for (const key in set.headers) (response as Response).headers.append(key, set.headers[key] as any) - if ((response as Response).status !== set.status) - set.status = (response as Response).status + return response } export const mapResponse = ( @@ -311,7 +321,10 @@ export const mapResponse = ( return Response.json(response, set as any) case 'Response': - mergeResponseWithSetHeaders(response as Response, set) + response = mergeResponseWithSetHeaders( + response as Response, + set + ) if ( (response as Response).headers.get('transfer-encoding') === @@ -354,7 +367,10 @@ export const mapResponse = ( default: if (response instanceof Response) { - mergeResponseWithSetHeaders(response as Response, set) + response = mergeResponseWithSetHeaders( + response as Response, + set + ) if ( (response as Response).headers.get( @@ -496,7 +512,10 @@ export const mapEarlyResponse = ( return Response.json(response, set as any) case 'Response': - mergeResponseWithSetHeaders(response as Response, set) + response = mergeResponseWithSetHeaders( + response as Response, + set + ) if ( (response as Response).headers.get('transfer-encoding') === @@ -540,7 +559,10 @@ export const mapEarlyResponse = ( default: if (response instanceof Response) { - mergeResponseWithSetHeaders(response as Response, set) + response = mergeResponseWithSetHeaders( + response as Response, + set + ) if ( (response as Response).headers.get( diff --git a/src/compose.ts b/src/compose.ts index 381089e4..36424544 100644 --- a/src/compose.ts +++ b/src/compose.ts @@ -2279,13 +2279,13 @@ export const composeErrorHandler = (app: AnyElysia) => { }) fnLiteral += - `const set = context.set\n` + + `const set=context.set\n` + `let _r\n` + `if(!context.code)context.code=error.code??error[ERROR_CODE]\n` + - `if(!(context.error instanceof Error))context.error = error\n` + + `if(!(context.error instanceof Error))context.error=error\n` + `if(error instanceof ElysiaCustomStatusResponse){` + - `error.status = error.code\n` + - `error.message = error.response` + + `set.status=error.status=error.code\n` + + `error.message=error.response` + `}` if (adapter.declare) fnLiteral += adapter.declare @@ -2314,7 +2314,7 @@ export const composeErrorHandler = (app: AnyElysia) => { `error.status=error.code\n` + `error.message = error.response` + `}` + - `if(set.status === 200) set.status = error.status\n` + `if(set.status===200||!set.status)set.status=error.status\n` const mapResponseReporter = report('mapResponse', { total: hooks.mapResponse.length, @@ -2348,19 +2348,23 @@ export const composeErrorHandler = (app: AnyElysia) => { fnLiteral += `if(error.constructor.name==="ValidationError"||error.constructor.name==="TransformDecodeError"){` + `if(error.error)error=error.error\n` + - `set.status = error.status??422\n` + + `set.status=error.status??422\n` + adapter.validationError + - `}else{` + - `if(error.code&&typeof error.status==="number"){` + - adapter.unknownError + - '}' + `}` + + fnLiteral += `if(error instanceof Error){` + adapter.unknownError + `}` const mapResponseReporter = report('mapResponse', { total: hooks.mapResponse.length, name: 'context' }) + fnLiteral += + '\nif(!context.response)context.response=error.message??error\n' + if (hooks.mapResponse.length) { + fnLiteral += 'let mr\n' + for (let i = 0; i < hooks.mapResponse.length; i++) { const mapResponse = hooks.mapResponse[i] @@ -2369,10 +2373,10 @@ export const composeErrorHandler = (app: AnyElysia) => { ) fnLiteral += - `context.response=error\n` + - `error=${ - isAsyncName(mapResponse) ? 'await ' : '' - }onMapResponse[${i}](context)\n` + `if(mr===undefined){` + + `mr=${isAsyncName(mapResponse) ? 'await ' : ''}onMapResponse[${i}](context)\n` + + `if(mr!==undefined)error=context.response=mr` + + '}' endUnit() } @@ -2380,7 +2384,7 @@ export const composeErrorHandler = (app: AnyElysia) => { mapResponseReporter.resolve() - fnLiteral += `\nreturn mapResponse(${saveResponse}error,set${adapter.mapResponseContext})}}` + fnLiteral += `\nreturn mapResponse(${saveResponse}error,set${adapter.mapResponseContext})}` return Function( 'inject', diff --git a/src/dynamic-handle.ts b/src/dynamic-handle.ts index 3150e343..76a0e81e 100644 --- a/src/dynamic-handle.ts +++ b/src/dynamic-handle.ts @@ -30,8 +30,8 @@ const injectDefaultValues = ( typeChecker: TypeCheck, obj: Record ) => { - // @ts-expect-error private for (const [key, keySchema] of Object.entries( + // @ts-expect-error private typeChecker.schema.properties )) { // @ts-expect-error private @@ -459,8 +459,11 @@ export const createDynamicHandler = (app: AnyElysia) => { error instanceof TransformDecodeError && error.error ? error.error : error - if ((reportedError as ElysiaErrors).status) - set.status = (reportedError as ElysiaErrors).status + + // ? Since error is reconciled in mergeResponseWithHeaders, this is not needed (if I'm not drunk) + // if ((reportedError as ElysiaErrors).status) + // set.status = (reportedError as ElysiaErrors).status + // @ts-expect-error private return app.handleError(context, reportedError) } finally { diff --git a/src/types.ts b/src/types.ts index de43aa93..61a737db 100644 --- a/src/types.ts +++ b/src/types.ts @@ -750,17 +750,29 @@ export type MapResponse< resolve: {} }, Path extends string | undefined = undefined -> = Handler< - Omit & { - response: {} extends Route['response'] ? unknown : Route['response'] - }, - Singleton & { - derive: { - response: Route['response'] - } - }, - Path -> +> = ( + context: Context< + Omit & {}, + Singleton & { + derive: { + response: {} extends Route['response'] + ? unknown + : Route['response'] + } + }, + Path + > +) => MaybePromise + +// Handler< +// Omit & {}, +// Singleton & { +// derive: { +// response: {} extends Route['response'] ? unknown : Route['response'] +// } +// }, +// Path +// > export type VoidHandler< in out Route extends RouteSchema = {}, diff --git a/test/core/handle-error.test.ts b/test/core/handle-error.test.ts index cdead8fd..6fee2d4a 100644 --- a/test/core/handle-error.test.ts +++ b/test/core/handle-error.test.ts @@ -70,9 +70,7 @@ describe('Handle Error', () => { }) it('inject headers to error', async () => { - const app = new Elysia({ - forceErrorEncapsulation: true - }) + const app = new Elysia() .onRequest(({ set }) => { set.headers['Access-Control-Allow-Origin'] = '*' }) @@ -248,4 +246,64 @@ describe('Handle Error', () => { expect(response.status).toEqual(404) expect(await response.text()).toEqual('foo') }) + + it('map status error to response', async () => { + const value = { message: 'meow!' } + + const response: Response = await new Elysia() + .get('/', () => 'Hello', { + beforeHandle({ error }) { + throw error("I'm a teapot", { message: 'meow!' }) + } + }) + // @ts-expect-error private property + .handleError( + { + request: new Request('http://localhost/'), + set: { + headers: {} + } + }, + error(422, value) as any + ) + + expect(await response.json()).toEqual(value) + expect(response.headers.get('content-type')).toStartWith( + 'application/json' + ) + expect(response.status).toEqual(422) + }) + + it('map status error with custom mapResponse', async () => { + const value = { message: 'meow!' } + + const response: Response = await new Elysia() + .mapResponse(({ response }) => { + if (typeof response === 'object') + return new Response('Don Quixote', { + headers: { + 'content-type': 'text/plain' + } + }) + }) + .get('/', () => 'Hello', { + beforeHandle({ error }) { + throw error("I'm a teapot", { message: 'meow!' }) + } + }) + // @ts-expect-error private property + .handleError( + { + request: new Request('http://localhost/'), + set: { + headers: {} + } + }, + error(422, value) as any + ) + + expect(await response.text()).toBe('Don Quixote') + expect(response.headers.get('content-type')).toStartWith('text/plain') + expect(response.status).toEqual(422) + }) })