Got: Got.Stream Hangs When POST/PUT/etc... Without Body

[thedillonb]

Describe the bug

• Node.js version: v10.15.3
• OS & version: OSX 10.15.4

Actual behavior

When executing a POST with got.stream with no body specified the result hangs until the socket is closed.

Expected behavior

The response, or error, should be propagated. If I do: { body: "" } or { body: Buffer.from([]) } it does work as intended.

got.post('https://google.com').then(...)

works just fine too.

Code to reproduce

const s = got.stream('https://google.com', { method: 'post' });
s.pipe(process.stdout);
s.on('error', err => console.error(err));
s.on('end', () => {
        console.log('all done');
});

Checklist

• [x] I have read the documentation.
• [x] I have tried my code with the latest version of Node.js and Got.

[Giotino]

It's an expected behavior, but I can stand by your side by saying that it's poorly documented.

The only reference to this behavior is in a comment of an example here

For POST, PUT, and PATCH methods got.stream returns a stream.Writable

@szmarczak @sindresorhus I think this need more documentation.

[thedillonb]

Oof. I get the idea but this kinda stinks. It makes using got.stream as a proxy need more logic about what method is being used and whether a body can be used. I would think it would be much better to make a uniform API, with a consistent return type rather than switching on the method used. In fact, It's not necessarily true that things like a GET can't have a request body. Yes, it's not great and the server may reject it, but that should not be for the client to prevent. https://stackoverflow.com/a/983458 Has a bunch of good info, but most importantly the latest spec:

Request message framing is independent of method semantics, even if the method doesn't define any use for a message body

In fact, if you wanted to use got with something like ElasticSearch you'd be out of luck: https://www.elastic.co/guide/en/elasticsearch/guide/current/_empty_search.html as it uses GET bodies.

I suggest got.stream always return a stream.Writable or always being able to take a body.

[thedillonb]

Any update on this, as, if the decision is to make the change, it will have been easier to break the API earlier than later.

[szmarczak]

@thedillonb The Elastic search docs also states:

However, because GET with a request body is not universally supported, the search API also accepts POST requests

If you aren't satisfied anyway, you can set the allowGetBody option to true but please note that it does not have any effect when using HTTP/2.

I'll look into this issue tomorrow as it's late here.