From 613698f39811ad1b6d2c84e0d864ede8e7540148 Mon Sep 17 00:00:00 2001 From: Meno Abels Date: Tue, 11 Oct 2022 18:53:46 +0200 Subject: [PATCH 1/7] return a proper error if middleware or api/route return not a falsy or Response value. --- packages/next/server/web/adapter.ts | 14 +++ .../edge-runtime-response-error/lib.js | 1 + .../edge-runtime-response-error/middleware.js | 6 ++ .../pages/api/route.js | 5 ++ .../pages/index.js | 3 + .../test/index.test.js | 88 +++++++++++++++++++ 6 files changed, 117 insertions(+) create mode 100644 test/integration/edge-runtime-response-error/lib.js create mode 100644 test/integration/edge-runtime-response-error/middleware.js create mode 100644 test/integration/edge-runtime-response-error/pages/api/route.js create mode 100644 test/integration/edge-runtime-response-error/pages/index.js create mode 100644 test/integration/edge-runtime-response-error/test/index.test.js diff --git a/packages/next/server/web/adapter.ts b/packages/next/server/web/adapter.ts index da29df6450fd4..9689164d40ec9 100644 --- a/packages/next/server/web/adapter.ts +++ b/packages/next/server/web/adapter.ts @@ -103,6 +103,20 @@ export async function adapter(params: { const event = new NextFetchEvent({ request, page: params.page }) let response = await params.handler(request, event) + // check if response is a Response object + if ( + response && + !( + typeof response === 'object' && + (!response.headers || typeof response.headers.get === 'function') && + (!response.body || typeof response.body.getReader === 'function') && + typeof response.status === 'number' && + typeof response.statusText === 'string' + ) + ) { + throw new Error('Response must be an instance of Response type') + } + /** * For rewrites we must always include the locale in the final pathname * so we re-create the NextURL forcing it to include it when the it is diff --git a/test/integration/edge-runtime-response-error/lib.js b/test/integration/edge-runtime-response-error/lib.js new file mode 100644 index 0000000000000..8cf56ac11d991 --- /dev/null +++ b/test/integration/edge-runtime-response-error/lib.js @@ -0,0 +1 @@ +// populated with tests diff --git a/test/integration/edge-runtime-response-error/middleware.js b/test/integration/edge-runtime-response-error/middleware.js new file mode 100644 index 0000000000000..172672681fb93 --- /dev/null +++ b/test/integration/edge-runtime-response-error/middleware.js @@ -0,0 +1,6 @@ +// populated with tests +export default () => { + return 'Boom' +} + +export const config = { matcher: '/' } diff --git a/test/integration/edge-runtime-response-error/pages/api/route.js b/test/integration/edge-runtime-response-error/pages/api/route.js new file mode 100644 index 0000000000000..a1ff06c9a8fc6 --- /dev/null +++ b/test/integration/edge-runtime-response-error/pages/api/route.js @@ -0,0 +1,5 @@ +export default async function handler(request) { + return 'Boom' +} + +export const config = { runtime: 'experimental-edge' } diff --git a/test/integration/edge-runtime-response-error/pages/index.js b/test/integration/edge-runtime-response-error/pages/index.js new file mode 100644 index 0000000000000..c5cc676685b67 --- /dev/null +++ b/test/integration/edge-runtime-response-error/pages/index.js @@ -0,0 +1,3 @@ +export default function Page() { + return
ok
+} diff --git a/test/integration/edge-runtime-response-error/test/index.test.js b/test/integration/edge-runtime-response-error/test/index.test.js new file mode 100644 index 0000000000000..86dae9f997084 --- /dev/null +++ b/test/integration/edge-runtime-response-error/test/index.test.js @@ -0,0 +1,88 @@ +/* eslint-disable jest/no-identical-title */ +/* eslint-env jest */ + +import { remove } from 'fs-extra' +import { join } from 'path' +import { + fetchViaHTTP, + File, + findPort, + killApp, + launchApp, + nextBuild, + nextStart, +} from 'next-test-utils' + +jest.setTimeout(1000 * 60 * 2) + +const context = { + appDir: join(__dirname, '../'), + logs: { output: '', stdout: '', stderr: '' }, + api: new File(join(__dirname, '../pages/api/route.js')), + lib: new File(join(__dirname, '../lib.js')), + middleware: new File(join(__dirname, '../middleware.js')), + page: new File(join(__dirname, '../pages/index.js')), +} +const appOption = { + env: { __NEXT_TEST_WITH_DEVTOOL: 1 }, + onStdout(msg) { + context.logs.output += msg + context.logs.stdout += msg + }, + onStderr(msg) { + context.logs.output += msg + context.logs.stderr += msg + }, +} +const routeUrl = '/api/route' +const middlewareUrl = '/' + +describe('Edge runtime code with imports', () => { + beforeEach(async () => { + context.appPort = await findPort() + context.logs = { output: '', stdout: '', stderr: '' } + await remove(join(__dirname, '../.next')) + }) + + afterEach(() => { + if (context.app) { + killApp(context.app) + } + context.api.restore() + context.middleware.restore() + context.lib.restore() + context.page.restore() + }) + + describe.each([ + { + title: 'Edge API', + url: routeUrl, + }, + { + title: 'Middleware', + url: middlewareUrl, + }, + ])('test error if response is not Response type', ({ title, url }) => { + it(`${title} dev test Response`, async () => { + context.app = await launchApp(context.appDir, context.appPort, appOption) + const res = await fetchViaHTTP(context.appPort, url) + expect(context.logs.stderr).toContain( + 'Response must be an instance of Response type' + ) + expect(res.status).toBe(500) + }) + + it(`${title} build test Response`, async () => { + await nextBuild(context.appDir, undefined, { + stderr: true, + }) + context.app = await nextStart(context.appDir, context.appPort, appOption) + const res = await fetchViaHTTP(context.appPort, url) + expect(context.logs.stderr).toContain( + 'Response must be an instance of Response type' + ) + expect(res.status).toBe(500) + }) + }) +}) From f72c7f9871ef4930995b4d020fa92bbe7467f4b4 Mon Sep 17 00:00:00 2001 From: Meno Abels Date: Tue, 11 Oct 2022 20:45:18 +0200 Subject: [PATCH 2/7] Update packages/next/server/web/adapter.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Balázs Orbán --- packages/next/server/web/adapter.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/next/server/web/adapter.ts b/packages/next/server/web/adapter.ts index 9689164d40ec9..24bfec9e65ce3 100644 --- a/packages/next/server/web/adapter.ts +++ b/packages/next/server/web/adapter.ts @@ -114,7 +114,7 @@ export async function adapter(params: { typeof response.statusText === 'string' ) ) { - throw new Error('Response must be an instance of Response type') + throw new TypeError('Response must be an instance of Response type') } /** From 101656ad6d40086b016aaad31c37372e5c56e621 Mon Sep 17 00:00:00 2001 From: Meno Abels Date: Tue, 11 Oct 2022 20:46:39 +0200 Subject: [PATCH 3/7] Update test/integration/edge-runtime-response-error/test/index.test.js MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Balázs Orbán --- test/integration/edge-runtime-response-error/test/index.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/edge-runtime-response-error/test/index.test.js b/test/integration/edge-runtime-response-error/test/index.test.js index 86dae9f997084..0b24a3b6f0da9 100644 --- a/test/integration/edge-runtime-response-error/test/index.test.js +++ b/test/integration/edge-runtime-response-error/test/index.test.js @@ -80,7 +80,7 @@ describe('Edge runtime code with imports', () => { context.app = await nextStart(context.appDir, context.appPort, appOption) const res = await fetchViaHTTP(context.appPort, url) expect(context.logs.stderr).toContain( - 'Response must be an instance of Response type' + 'Expected an instance of Response to be returned' ) expect(res.status).toBe(500) }) From 42159d93fc6cbe03aa4996fd3c102723192df8e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Orb=C3=A1n?= Date: Tue, 11 Oct 2022 20:55:53 +0200 Subject: [PATCH 4/7] Apply suggestions from code review --- packages/next/server/web/adapter.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/next/server/web/adapter.ts b/packages/next/server/web/adapter.ts index 24bfec9e65ce3..e659ebfae5868 100644 --- a/packages/next/server/web/adapter.ts +++ b/packages/next/server/web/adapter.ts @@ -114,7 +114,7 @@ export async function adapter(params: { typeof response.statusText === 'string' ) ) { - throw new TypeError('Response must be an instance of Response type') + throw new TypeError('Expected an instance of Response to be returned') } /** From 7e5a911a450387cd0f217855ef6be1302069df6e Mon Sep 17 00:00:00 2001 From: Meno Abels Date: Tue, 11 Oct 2022 21:05:41 +0200 Subject: [PATCH 5/7] update error message --- test/integration/edge-runtime-response-error/test/index.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/edge-runtime-response-error/test/index.test.js b/test/integration/edge-runtime-response-error/test/index.test.js index 0b24a3b6f0da9..ad57562801e83 100644 --- a/test/integration/edge-runtime-response-error/test/index.test.js +++ b/test/integration/edge-runtime-response-error/test/index.test.js @@ -68,7 +68,7 @@ describe('Edge runtime code with imports', () => { context.app = await launchApp(context.appDir, context.appPort, appOption) const res = await fetchViaHTTP(context.appPort, url) expect(context.logs.stderr).toContain( - 'Response must be an instance of Response type' + 'Expected an instance of Response to be returned' ) expect(res.status).toBe(500) }) From ee9d999cad2cc28be6a12e9b0500a713dc81795a Mon Sep 17 00:00:00 2001 From: Meno Abels Date: Wed, 12 Oct 2022 09:09:51 +0200 Subject: [PATCH 6/7] Update packages/next/server/web/adapter.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Balázs Orbán --- packages/next/server/web/adapter.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/packages/next/server/web/adapter.ts b/packages/next/server/web/adapter.ts index e659ebfae5868..b6befaf354ef8 100644 --- a/packages/next/server/web/adapter.ts +++ b/packages/next/server/web/adapter.ts @@ -106,13 +106,7 @@ export async function adapter(params: { // check if response is a Response object if ( response && - !( - typeof response === 'object' && - (!response.headers || typeof response.headers.get === 'function') && - (!response.body || typeof response.body.getReader === 'function') && - typeof response.status === 'number' && - typeof response.statusText === 'string' - ) + !(response instanceof Response) ) { throw new TypeError('Expected an instance of Response to be returned') } From 9ae16871657d12a8a09b493322b4b889490fded9 Mon Sep 17 00:00:00 2001 From: Meno Abels Date: Wed, 12 Oct 2022 09:33:12 +0200 Subject: [PATCH 7/7] make prettier happy --- packages/next/server/web/adapter.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/next/server/web/adapter.ts b/packages/next/server/web/adapter.ts index a3ebb44c679c9..49a8ec2b52887 100644 --- a/packages/next/server/web/adapter.ts +++ b/packages/next/server/web/adapter.ts @@ -107,10 +107,7 @@ export async function adapter(params: { let response = await params.handler(request, event) // check if response is a Response object - if ( - response && - !(response instanceof Response) - ) { + if (response && !(response instanceof Response)) { throw new TypeError('Expected an instance of Response to be returned') }