Skip to content

Commit

Permalink
Add warning when API resolves without the request being finished (#9999)
Browse files Browse the repository at this point in the history
* Add error when API resolves without the request being finished

* Update to only show warning in development instead

* Update packages/next/next-server/server/api-utils.ts

Co-authored-by: Joe Haddad <timer150@gmail.com>
  • Loading branch information
ijjk and Timer committed Jan 10, 2020
1 parent dd020fb commit 14ca20d
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 10 deletions.
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.`
)
}
} 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

0 comments on commit 14ca20d

Please sign in to comment.