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

Add warning when API resolves without the request being finished #9999

Merged
merged 4 commits into from
Jan 10, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions packages/next/next-server/server/api-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { parse } from 'content-type'
import { Params } from './router'
import { PageConfig } from '../../types'
import { interopDefault } from './load-components'
import { isResSent } from '../lib/utils'

export type NextApiRequestCookies = { [key: string]: string }
export type NextApiRequestQuery = { [key: string]: string | string[] }
Expand Down Expand Up @@ -55,6 +56,12 @@ export async function apiResolver(

const resolver = interopDefault(resolverModule)
await resolver(req, res)

if (process.env.NODE_ENV !== 'production' && !isResSent(res)) {
console.warn(
`API resolved without sending a response for ${req.url}, this may result in a stalled requests`
Timer marked this conversation as resolved.
Show resolved Hide resolved
)
}
} catch (err) {
if (err instanceof ApiError) {
sendError(apiRes, err.statusCode, err.message)
Expand Down
21 changes: 12 additions & 9 deletions test/integration/api-support/pages/api/no-parsing.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,17 @@ export const config = {
}

export default (req, res) => {
if (!req.body) {
let buffer = ''
req.on('data', chunk => {
buffer += chunk
})
return new Promise(resolve => {
if (!req.body) {
let buffer = ''
req.on('data', chunk => {
buffer += chunk
})

req.on('end', () => {
res.status(200).json(JSON.parse(Buffer.from(buffer).toString()))
})
}
req.on('end', () => {
res.status(200).json(JSON.parse(Buffer.from(buffer).toString()))
resolve()
})
}
})
}
4 changes: 4 additions & 0 deletions test/integration/api-support/pages/api/test-no-end.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export default (req, res) => {
console.log('hi')
return {}
}
22 changes: 21 additions & 1 deletion test/integration/api-support/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
/* global jasmine */
import fs from 'fs-extra'
import { join } from 'path'
import AbortController from 'abort-controller'
import {
killApp,
findPort,
Expand All @@ -17,6 +18,7 @@ jasmine.DEFAULT_TIMEOUT_INTERVAL = 1000 * 60 * 2
const appDir = join(__dirname, '../')
const nextConfig = join(appDir, 'next.config.js')
let appPort
let stderr
let mode
let app

Expand Down Expand Up @@ -332,6 +334,19 @@ function runTests(dev = false) {
)
).toBeTruthy()
})

it('should show warning when the API resolves without ending the request in dev mode', async () => {
const controller = new AbortController()
setTimeout(() => {
controller.abort()
}, 2000)
await fetchViaHTTP(appPort, '/api/test-no-end', undefined, {
signal: controller.signal,
}).catch(() => {})
expect(stderr).toContain(
`API resolved without sending a response for /api/test-no-end, this may result in a stalled requests`
)
})
} else {
it('should build api routes', async () => {
const pagesManifest = JSON.parse(
Expand Down Expand Up @@ -360,8 +375,13 @@ function runTests(dev = false) {
describe('API routes', () => {
describe('dev support', () => {
beforeAll(async () => {
stderr = ''
appPort = await findPort()
app = await launchApp(appDir, appPort)
app = await launchApp(appDir, appPort, {
onStderr: msg => {
stderr += msg
},
})
})
afterAll(() => killApp(app))

Expand Down