diff --git a/source/core/index.ts b/source/core/index.ts index a6fc9ff33..20a3e2f4d 100644 --- a/source/core/index.ts +++ b/source/core/index.ts @@ -1,5 +1,5 @@ import {promisify} from 'util'; -import {Duplex, Readable} from 'stream'; +import {Duplex, Writable, Readable} from 'stream'; import {ReadStream} from 'fs'; import {URL, URLSearchParams} from 'url'; import {Socket} from 'net'; @@ -56,6 +56,8 @@ const kOriginalResponse = Symbol('originalResponse'); const kRetryTimeout = Symbol('retryTimeout'); export const kIsNormalizedAlready = Symbol('isNormalizedAlready'); +const destroyedWithoutErrorCodes = new Set(['ECANCELED', 'ERR_STREAM_DESTROYED']); + const supportsBrotli = is.string((process.versions as any).brotli); export interface Agents { @@ -1400,6 +1402,25 @@ export default class Request extends Duplex implements RequestEvents { // TODO: Remove this when targeting Node.js >= 12 this._progressCallbacks = []; + const unlockWrite = (): void => this._unlockWrite(); + const lockWrite = (): void => this._lockWrite(); + + this.on('pipe', (source: Writable) => { + source.prependListener('data', unlockWrite); + source.on('data', lockWrite); + + source.prependListener('end', unlockWrite); + source.on('end', lockWrite); + }); + + this.on('unpipe', (source: Writable) => { + source.off('data', unlockWrite); + source.off('data', lockWrite); + + source.off('end', unlockWrite); + source.off('end', lockWrite); + }); + this.on('pipe', source => { if (source instanceof IncomingMessage) { this.options.headers = { @@ -1409,6 +1430,11 @@ export default class Request extends Duplex implements RequestEvents { } }); + const {json, body, form} = options; + if (json || body || form) { + this._lockWrite(); + } + if (kIsNormalizedAlready in options) { this.options = options as NormalizedOptions; } else { @@ -1861,6 +1887,30 @@ export default class Request extends Duplex implements RequestEvents { return normalizePromiseArguments(options as NormalizedOptions, defaults); } + _lockWrite(): void { + const onLockedWrite = (): never => { + throw new TypeError('The payload has been already provided'); + }; + + // `stream.pipeline()` ends the destination from its own `end` listener, after the lock is back in place (Node.js 17.3+). Ending without a payload writes nothing, so it stays allowed. + const endWithoutPayload = (...args: unknown[]): this => { + const [chunk] = args; + if (chunk !== undefined && chunk !== null && typeof chunk !== 'function') { + onLockedWrite(); + } + + return Duplex.prototype.end.apply(this, args as Parameters) as this; + }; + + this.write = onLockedWrite; + this.end = endWithoutPayload; + } + + _unlockWrite(): void { + this.write = super.write; + this.end = super.end; + } + async _finalizeBody(): Promise { const {options} = this; const {headers} = options; @@ -1938,6 +1988,10 @@ export default class Request extends Duplex implements RequestEvents { } } } + } else if (cannotHaveBody) { + this._lockWrite(); + } else { + this._unlockWrite(); } this[kBodySize] = Number(headers['content-length']) || undefined; @@ -2205,11 +2259,19 @@ export default class Request extends Duplex implements RequestEvents { body.once('error', (error: NodeJS.ErrnoException) => { this._beforeError(new UploadError(error, this)); }); - } else if (!is.undefined(body)) { - this._writeRequest(body, undefined, () => {}); - currentRequest.end(); - } else if (this._cannotHaveBody || this._noPipe) { - currentRequest.end(); + } else { + this._unlockWrite(); + + if (!is.undefined(body)) { + this._writeRequest(body, undefined, () => {}); + currentRequest.end(); + + this._lockWrite(); + } else if (this._cannotHaveBody || this._noPipe) { + currentRequest.end(); + + this._lockWrite(); + } } this.emit('request', request); @@ -2441,7 +2503,9 @@ export default class Request extends Duplex implements RequestEvents { void this._onResponse(requestOrResponse as IncomingMessageWithTimings); }); + this._unlockWrite(); this.end(); + this._lockWrite(); } else { void this._onResponse(requestOrResponse as IncomingMessageWithTimings); } @@ -2646,9 +2710,19 @@ export default class Request extends Duplex implements RequestEvents { return; } - this[kRequest]!.end((error?: Error | null) => { + const request = this[kRequest]!; + + request.end((error?: NodeJS.ErrnoException | null) => { if (error) { // `ClientRequest.end()` can report the same failure as the request's `error` event. Route it through Got's retry handling without completing `_final`, so this Duplex does not finish a failed upload. + // A request destroyed without an error reports `ECANCELED` here before its `error` event carries the retryable cause, and `close` always follows that event. + if (destroyedWithoutErrorCodes.has(error.code!) && !(request as ClientRequest & {closed?: boolean}).closed) { + request.once('close', () => { + this._beforeError(error); + }); + return; + } + this._beforeError(error); return; } diff --git a/test/retry.ts b/test/retry.ts index 885a9765d..db0a9dbb2 100644 --- a/test/retry.ts +++ b/test/retry.ts @@ -48,6 +48,44 @@ const createRequestWithEndError = (scenario: RequestEndErrorScenario): http.Clie return request; }; +const createDestroyedRequest = ({emitsError, closed = false}: {emitsError: boolean; closed?: boolean}): http.ClientRequest => { + const request = new EventEmitter() as http.ClientRequest & {closed: boolean}; + request.closed = closed; + + // @ts-expect-error Mocking the behaviour of a ClientRequest + request.write = (_chunk: unknown, _encoding: unknown, callback?: () => void) => { + process.nextTick(() => callback?.()); + return true; + }; + + // @ts-expect-error Mocking the behaviour of a ClientRequest + request.end = (callback: (error: Error) => void) => { + process.nextTick(() => { + callback(Object.assign(new Error('write ECANCELED'), {code: 'ECANCELED'})); + + if (closed) { + return; + } + + if (emitsError) { + request.emit('error', Object.assign(new Error('socket hang up'), {code: 'ECONNRESET'})); + } + + request.closed = true; + request.emit('close'); + }); + }; + + request.abort = () => {}; + + request.destroy = () => { + request.destroyed = true; + return request; + }; + + return request; +}; + const retryImmediately = { calculateDelay: ({computedValue}: {computedValue: number}) => computedValue === 0 ? 0 : 1 }; @@ -382,6 +420,116 @@ test('retries a refused connection and rejects with the connection error', async t.is(error.request?.retryCount, 2); }); +test('retries the request error that follows an `ECANCELED` end callback', async t => { + let attemptCount = 0; + const retries: Array = []; + + const error = await t.throwsAsync(got.put('http://localhost', { + body: 'wow', + request: () => { + attemptCount++; + return createDestroyedRequest({emitsError: true}); + }, + retry: { + limit: 2, + ...retryImmediately + }, + hooks: { + beforeRetry: [ + (_options, error) => { + retries.push(error?.code); + } + ] + } + }), { + instanceOf: RequestError + }); + + t.is(error.code, 'ECONNRESET'); + t.is(attemptCount, 3); + t.deepEqual(retries, ['ECONNRESET', 'ECONNRESET']); +}); + +test('rejects with `ECANCELED` when the destroyed request emits no error', async t => { + let attemptCount = 0; + + const error = await t.throwsAsync(got.put('http://localhost', { + body: 'wow', + request: () => { + attemptCount++; + return createDestroyedRequest({emitsError: false}); + }, + retry: { + limit: 2, + ...retryImmediately + } + }), { + instanceOf: RequestError + }); + + t.is(error.code, 'ECANCELED'); + t.is(attemptCount, 1); +}); + +test('rejects with `ECANCELED` when the request already closed', async t => { + const error = await t.throwsAsync(got.put('http://localhost', { + body: 'wow', + request: () => createDestroyedRequest({emitsError: false, closed: true}), + retry: 0 + }), { + instanceOf: RequestError + }); + + t.is(error.code, 'ECANCELED'); +}); + +test('retries an upload whose socket is destroyed without an error', async t => { + const server = http.createServer(request => { + request.pause(); + }); + + await new Promise(resolve => { + server.listen(0, '127.0.0.1', resolve); + }); + + t.teardown(() => { + server.close(); + }); + + const {port} = server.address() as {port: number}; + const retries: Array = []; + + const error = await t.throwsAsync(got.put(`http://127.0.0.1:${port}`, { + body: Buffer.alloc(64 * 1024 * 1024), + agent: {http: new http.Agent({keepAlive: false})}, + request: (url, options, callback) => { + const request = http.request(url, options, callback); + request.once('socket', socket => { + setTimeout(() => { + socket.destroy(); + }, 100); + }); + return request; + }, + retry: { + limit: 1, + ...retryImmediately + }, + hooks: { + beforeRetry: [ + (_options, error) => { + retries.push(error?.code); + } + ] + } + }), { + instanceOf: RequestError + }); + + t.is(error.code, 'ECONNRESET'); + t.deepEqual(retries, ['ECONNRESET']); +}); + test('respects 413 Retry-After', withServer, async (t, server, got) => { let lastTried413access = Date.now(); server.get('/', (_request, response) => { diff --git a/test/stream.ts b/test/stream.ts index e2069423c..fa198f5e9 100644 --- a/test/stream.ts +++ b/test/stream.ts @@ -2,6 +2,8 @@ import {promisify} from 'util'; import fs = require('fs'); import {PassThrough as PassThroughStream} from 'stream'; import stream = require('stream'); +import http = require('http'); +import {AddressInfo} from 'net'; import test from 'ava'; import {Handler} from 'express'; import toReadableStream = require('to-readable-stream'); @@ -69,6 +71,88 @@ test('returns writeable stream', withServer, async (t, server, got) => { t.is(await promise, 'wow'); }); +test('throws on write if body is specified', withServer, (t, server, got) => { + server.post('/', postHandler); + + const streams = [ + got.stream.post({body: 'wow'}), + got.stream.post({json: {}}), + got.stream.post({form: {}}) + ]; + + for (const stream of streams) { + t.throws(() => { + stream.end('wow'); + }, { + message: 'The payload has been already provided' + }); + + stream.destroy(); + } +}); + +test('throws on write if no payload method is present', withServer, (t, server, got) => { + server.post('/', postHandler); + + const stream = got.stream.get(''); + + t.throws(() => { + stream.end('wow'); + }, { + message: 'The payload has been already provided' + }); + + stream.destroy(); +}); + +test('data written after the body is never sent', async t => { + const requests: string[] = []; + const server = http.createServer((request, response) => { + let body = ''; + request.setEncoding('utf8'); + request.on('data', (chunk: string) => { + body += chunk; + }); + request.on('end', () => { + requests.push(`${request.method!} ${request.url!} ${body}`); + response.end('ok'); + }); + }); + + await new Promise(resolve => { + server.listen(0, '127.0.0.1', resolve); + }); + t.teardown(() => { + server.close(); + }); + + const {port} = server.address() as AddressInfo; + const stream = got.stream.post(`http://127.0.0.1:${port}/upload`, {body: 'abc'}); + + t.throws(() => { + stream.write('GET /injected HTTP/1.1\r\nHost: localhost\r\n\r\n'); + }, { + message: 'The payload has been already provided' + }); + + t.is(await getStream(stream), 'ok'); + t.deepEqual(requests, ['POST /upload abc']); +}); + +test('ending a stream without a payload does not throw', withServer, async (t, server, got) => { + server.get('/', (_request, response) => { + response.end('ok'); + }); + + const stream = got.stream(''); + + t.notThrows(() => { + stream.end(); + }); + + t.is(await getStream(stream), 'ok'); +}); + test('does not throw if using stream and passing a json option', withServer, async (t, server, got) => { server.post('/', postHandler);