Skip to content

Commit 4d5168c

Browse files
committed
Fix stream auto-end for empty PATCH/DELETE/OPTIONS
Fixes #2429
1 parent 388df48 commit 4d5168c

File tree

4 files changed

+22
-4
lines changed

4 files changed

+22
-4
lines changed

documentation/3-streams.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ This constructor takes the same arguments as the Got promise.
2626
**Note:**
2727
> - While `got.post('https://example.com')` resolves, `got.stream.post('https://example.com')` will hang indefinitely until a body is provided.
2828
> - If there's no body on purpose, remember to `stream.end()` or set the body option to an empty string.
29+
> - `got.stream` does not auto-end for `OPTIONS`, `DELETE`, or `PATCH` so you can pipe or write a body without getting `write after end`. Call `stream.end()` when you are not piping a body.
2930
3031
```js
3132
import stream from 'node:stream';

source/core/index.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,6 @@ const supportsBrotli = is.string(process.versions.brotli);
6767
const supportsZstd = is.string(process.versions.zstd);
6868

6969
const methodsWithoutBody: ReadonlySet<string> = new Set(['GET', 'HEAD']);
70-
// Methods that should auto-end streams when no body is provided
71-
const methodsWithoutBodyStream: ReadonlySet<string> = new Set(['OPTIONS', 'DELETE', 'PATCH']);
7270

7371
export type GotEventFunction<T> =
7472
/**
@@ -1224,9 +1222,8 @@ export default class Request extends Duplex implements RequestEvents<Request> {
12241222
} else if (is.undefined(body)) {
12251223
// No body to send, end the request
12261224
const cannotHaveBody = methodsWithoutBody.has(this.options.method) && !(this.options.method === 'GET' && this.options.allowGetBody);
1227-
const shouldAutoEndStream = methodsWithoutBodyStream.has(this.options.method);
12281225

1229-
if ((this._noPipe ?? false) || cannotHaveBody || currentRequest !== this || shouldAutoEndStream) {
1226+
if ((this._noPipe ?? false) || cannotHaveBody || currentRequest !== this) {
12301227
currentRequest.end();
12311228
}
12321229
} else {

source/types.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,8 @@ export type Got<GotOptions extends ExtendOptions = ExtendOptions> = {
250250
- uploadProgress
251251
- downloadProgress
252252
- error
253+
254+
Note: For writable request streams, call `stream.end()` when you are not piping a body. `got.stream` does not auto-end for OPTIONS, DELETE, or PATCH so you can pipe or write a body without getting `write after end`.
253255
*/
254256
stream: GotStream;
255257

test/stream.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -577,6 +577,7 @@ test('OPTIONS stream without body completes successfully', withServer, async (t,
577577
});
578578

579579
const stream = got.stream({method: 'OPTIONS'});
580+
stream.end();
580581
await t.notThrowsAsync(getStream(stream));
581582
});
582583

@@ -587,6 +588,7 @@ test('DELETE stream without body completes successfully', withServer, async (t,
587588
});
588589

589590
const stream = got.stream({method: 'DELETE'});
591+
stream.end();
590592
await t.notThrowsAsync(getStream(stream));
591593
});
592594

@@ -597,10 +599,26 @@ test('PATCH stream without body completes successfully', withServer, async (t, s
597599
});
598600

599601
const stream = got.stream({method: 'PATCH'});
602+
stream.end();
600603
const data = await getStream(stream);
601604
t.is(data, 'patched');
602605
});
603606

607+
test('PATCH stream waits for piped body', withServer, async (t, server, got) => {
608+
server.patch('/', postHandler);
609+
610+
const destination = new stream.PassThrough();
611+
const responsePromise = getStream(destination);
612+
613+
await streamPipeline(
614+
ReadableStream.from(['Hello, world!']),
615+
got.stream.patch(''),
616+
destination,
617+
);
618+
619+
t.is(await responsePromise, 'Hello, world!');
620+
});
621+
604622
test('throws error when content-length does not match bytes transferred - stream', withServer, async (t, server, got) => {
605623
server.get('/', (_request, response) => {
606624
response.writeHead(200, {

0 commit comments

Comments
 (0)