Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proper error if middleware or api/route not return a Response #41336

Merged
14 changes: 14 additions & 0 deletions packages/next/server/web/adapter.ts
Expand Up @@ -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'
)
mabels marked this conversation as resolved.
Show resolved Hide resolved
) {
throw new Error('Response must be an instance of Response type')
mabels marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* 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
Expand Down
1 change: 1 addition & 0 deletions test/integration/edge-runtime-response-error/lib.js
@@ -0,0 +1 @@
// populated with tests
6 changes: 6 additions & 0 deletions test/integration/edge-runtime-response-error/middleware.js
@@ -0,0 +1,6 @@
// populated with tests
export default () => {
return 'Boom'
}

export const config = { matcher: '/' }
@@ -0,0 +1,5 @@
export default async function handler(request) {
return 'Boom'
}

export const config = { runtime: 'experimental-edge' }
3 changes: 3 additions & 0 deletions test/integration/edge-runtime-response-error/pages/index.js
@@ -0,0 +1,3 @@
export default function Page() {
return <div>ok</div>
}
88 changes: 88 additions & 0 deletions 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'
mabels marked this conversation as resolved.
Show resolved Hide resolved
)
expect(res.status).toBe(500)
})
})
})