Skip to content

Commit

Permalink
fix: fix hanging on broken responses
Browse files Browse the repository at this point in the history
  • Loading branch information
wyattjoh committed Oct 17, 2023
1 parent 4b24c12 commit 14bee29
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 40 deletions.
7 changes: 2 additions & 5 deletions packages/next/src/server/base-http/web.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
import type { IncomingHttpHeaders, OutgoingHttpHeaders } from 'http'
import type { FetchMetrics } from './index'

// This takes advantage of `Promise.withResolvers` which is polyfilled in
// this imported module.
import '../../lib/polyfill-promise-with-resolvers'

import { toNodeOutgoingHttpHeaders } from '../web/utils'
import { BaseNextRequest, BaseNextResponse } from './index'
import { DetachedPromise } from '../../lib/detached-promise'

export class WebNextRequest extends BaseNextRequest<ReadableStream | null> {
public request: Request
Expand Down Expand Up @@ -87,7 +84,7 @@ export class WebNextResponse extends BaseNextResponse<WritableStream> {
return this
}

private readonly sendPromise = Promise.withResolvers<void>()
private readonly sendPromise = new DetachedPromise<void>()
private _sent = false
public send() {
this.sendPromise.resolve()
Expand Down
45 changes: 26 additions & 19 deletions packages/next/src/server/pipe-readable.ts
Original file line number Diff line number Diff line change
@@ -1,35 +1,37 @@
import type { ServerResponse } from 'node:http'

import './node-polyfill-web-streams'
import { abortControllerFromNodeResponse } from './web/spec-extension/adapters/next-request'

import { createAbortController } from './web/spec-extension/adapters/next-request'
import { DetachedPromise } from '../lib/detached-promise'

export function isAbortError(e: any): e is Error & { name: 'AbortError' } {
return e?.name === 'AbortError'
}

function createWriterFromResponse(
res: ServerResponse,
controller: AbortController
res: ServerResponse
): WritableStream<Uint8Array> {
let started = false

// Create a promise that will resolve once the response has drained. See
// https://nodejs.org/api/stream.html#stream_event_drain
let drained = Promise.withResolvers<void>()
res.on('drain', () => {
let drained = new DetachedPromise<void>()
function onDrain() {
drained.resolve()
})
}
res.on('drain', onDrain)

// Create a promise that will resolve once the response has finished. See
// https://nodejs.org/api/http.html#event-finish_1
const finished = Promise.withResolvers<void>()
// If the finish event fires, it means we shouldn't block and wait for the
// drain event.
res.once('close', () => {
// If the finish event fires, it means we shouldn't block and wait for the
// drain event.
res.off('drain', onDrain)
drained.resolve()
})

// Once the response finishes, resolve the promise.
// Create a promise that will resolve once the response has finished. See
// https://nodejs.org/api/http.html#event-finish_1
const finished = new DetachedPromise<void>()
res.once('finish', () => {
finished.resolve()
})
Expand Down Expand Up @@ -57,16 +59,21 @@ function createWriterFromResponse(
// If the write returns false, it means there's some backpressure, so
// wait until it's streamed before continuing.
if (!ok) {
// Reset the drained promise so that we can wait for the next drain event.
drained = Promise.withResolvers<void>()

await drained.promise

// Reset the drained promise so that we can wait for the next drain event.
drained = new DetachedPromise<void>()
}
} catch (err: any) {
controller.abort(err)
} catch (err) {
res.end()
throw err
}
},
abort: (err) => {
if (res.writableFinished) return

res.destroy(err)
},
close: () => {
if (res.writableFinished) return

Expand All @@ -87,9 +94,9 @@ export async function pipeToNodeResponse(

// Create a new AbortController so that we can abort the readable if the
// client disconnects.
const controller = abortControllerFromNodeResponse(res)
const controller = createAbortController(res)

const writer = createWriterFromResponse(res, controller)
const writer = createWriterFromResponse(res)

await readable.pipeTo(writer, { signal: controller.signal })
} catch (err: any) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,25 +7,24 @@ import { getRequestMeta } from '../../../request-meta'
import { fromNodeOutgoingHttpHeaders } from '../../utils'
import { NextRequest } from '../request'

export function abortControllerFromNodeResponse(
response: Writable
): AbortController {
/**
* Creates an AbortController tied to the closing of a ServerResponse (or other
* appropriate Writable).
*
* If the `close` event is fired before the `finish` event, then we'll send the
* `abort` signal.
*/
export function createAbortController(response: Writable): AbortController {
const controller = new AbortController()

// If `finish` fires first, then `res.end()` has been called and the close is
// just us finishing the stream on our side. If `close` fires first, then we
// know the client disconnected before we finished.
function onClose() {
controller.abort()
// eslint-disable-next-line @typescript-eslint/no-use-before-define
response.off('finish', onFinish)
}
function onFinish() {
response.off('close', onClose)
}
response.once('close', () => {
if (response.writableFinished) return

response.once('close', onClose)
response.once('finish', onFinish)
controller.abort()
})

return controller
}
Expand All @@ -42,9 +41,8 @@ export function signalFromNodeResponse(response: Writable): AbortSignal {
const { errored, destroyed } = response
if (errored || destroyed) return AbortSignal.abort(errored)

const controller = abortControllerFromNodeResponse(response)

return controller.signal
const { signal } = createAbortController(response)
return signal
}

export class NextRequestAdapter {
Expand Down

0 comments on commit 14bee29

Please sign in to comment.