diff --git a/cli/tests/unit/serve_test.ts b/cli/tests/unit/serve_test.ts index 9f6bd4aa16..59334039ad 100644 --- a/cli/tests/unit/serve_test.ts +++ b/cli/tests/unit/serve_test.ts @@ -3805,3 +3805,70 @@ Deno.test( await server.finished; }, ); + +// serve Handler must return Response class or promise that resolves Response class +Deno.test( + { permissions: { net: true, run: true } }, + async function handleServeCallbackReturn() { + const d = deferred(); + const listeningPromise = deferred(); + const ac = new AbortController(); + + const server = Deno.serve( + { + port: servePort, + onListen: onListen(listeningPromise), + signal: ac.signal, + onError: (error) => { + assert(error instanceof TypeError); + assert( + error.message === + "Return value from serve handler must be a response or a promise resolving to a response", + ); + d.resolve(); + return new Response("Customized Internal Error from onError"); + }, + }, + () => { + // Trick the typechecker + return undefined; + }, + ); + await listeningPromise; + const respText = await curlRequest([`http://localhost:${servePort}`]); + await d; + ac.abort(); + await server.finished; + assert(respText === "Customized Internal Error from onError"); + }, +); + +// onError Handler must return Response class or promise that resolves Response class +Deno.test( + { permissions: { net: true, run: true } }, + async function handleServeErrorCallbackReturn() { + const listeningPromise = deferred(); + const ac = new AbortController(); + + const server = Deno.serve( + { + port: servePort, + onListen: onListen(listeningPromise), + signal: ac.signal, + onError: () => { + // Trick the typechecker + return undefined; + }, + }, + () => { + // Trick the typechecker + return undefined; + }, + ); + await listeningPromise; + const respText = await curlRequest([`http://localhost:${servePort}`]); + ac.abort(); + await server.finished; + assert(respText === "Internal Server Error"); + }, +); diff --git a/ext/http/00_serve.js b/ext/http/00_serve.js index b99f28c0fb..5b931ccd1e 100644 --- a/ext/http/00_serve.js +++ b/ext/http/00_serve.js @@ -10,6 +10,7 @@ import { Event } from "ext:deno_web/02_event.js"; import { fromInnerResponse, newInnerResponse, + ResponsePrototype, toInnerResponse, } from "ext:deno_fetch/23_response.js"; import { fromInnerRequest, toInnerRequest } from "ext:deno_fetch/23_request.js"; @@ -449,15 +450,26 @@ function mapToCallback(context, callback, onError) { fromInnerRequest(innerRequest, signal, "immutable"), new ServeHandlerInfo(innerRequest), ); + + // Throwing Error if the handler return value is not a Response class + if (!ObjectPrototypeIsPrototypeOf(ResponsePrototype, response)) { + throw TypeError( + "Return value from serve handler must be a response or a promise resolving to a response", + ); + } } catch (error) { try { response = await onError(error); + if (!ObjectPrototypeIsPrototypeOf(ResponsePrototype, response)) { + throw TypeError( + "Return value from onError handler must be a response or a promise resolving to a response", + ); + } } catch (error) { console.error("Exception in onError while handling exception", error); response = internalServerError(); } } - const inner = toInnerResponse(response); if (innerRequest?.[_upgraded]) { // We're done here as the connection has been upgraded during the callback and no longer requires servicing.