Got: Server side aborted request in stream mode hang with node 14.3.0

[darksabrefr]

Describe the bug

• Node.js version: 14.3.0
• OS & version: Ubuntu 19

Server side aborted requests in stream mode conduce got to hang with node >= 14.3.0. In fact the aborted event from the IncomingMessage is not transmitted or transformed to an error in the stream got returns.

Code to reproduce

const http = require('http');
const got = require('got');

// http server that aborts a request
const server = http.createServer((req, res) => {
    res.on('close', () => {
        console.log('server close')
    })

    res.writeHead(200, { 'Content-Type': 'text/plain' });
    res.write('chunk 1');

    setTimeout(() => res.write('chunk 2'), 1000);
    setTimeout(() => res.destroy(), 2000);
});
server.listen(8000);

// got request
(async () => {
    try {
        const gotOptions = {timeout: {socket: 2000, request: 3000}, isStream: true};
        const httpRequest = got('http://localhost:8000', gotOptions);
        for await (const chunk of httpRequest) {
            console.log(`client data: aborted=${httpRequest.aborted}, data=${chunk.toString()}`)
        }
        console.log(`client stream end`);
    } catch (e) {
        console.log(`client error: error=${e}`);
    }
    console.log(`end`);
})();

Node output

14.3.0

client data: aborted=false, data=chunk 1
client data: aborted=false, data=chunk 2
server close

14.2.0

client data: aborted=false, data=chunk 1
client data: aborted=false, data=chunk 2
client error: error=ReadError: The server aborted the pending request
end
server close

Comments

With node 14.2.0 we can see when the request is aborted that an error in thrown from the for await loop. We can see the end message too signaling the request is terminated. With node 14.3.0, no error is thrown and the stream is not terminated, we are stuck in the for await loop

Workaround

It's possible to use this workaround to restore the previous behavior (but we get a got.RequestError and not more a got.ReadError):

const {once} = require('events');
const got = require('got');

(async () => {
    try {
        const gotOptions = {timeout: 3000, isStream: true};
        const httpRequest = got('http://localhost:8000', gotOptions);
        const [httpResponse] = await once(httpRequest, 'response');
        httpResponse.on('aborted', () => {
            httpRequest.destroy(new Error('The server aborted the pending request'));
        });
        for await (const chunk of httpRequest) {
            console.log(`client data: aborted=${httpRequest.aborted}, data=${chunk.toString()}`)
        }
        console.log(`client stream end`);
    } catch (e) {
        console.log(`client error: error=${e}`);
    }
    console.log(`end`);
})();

Checklist

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

[Giotino]

This is causing the test "throws an error if the server aborted the request" to fail.

These Node.JS commits (merged into 14.3.0) could be relevant to this issue:
nodejs/node@b634d4b
nodejs/node@cc5c8e0

[Giotino]

The problem seems to be with the aborted property of the got request:
https://github.com/sindresorhus/got/blob/master/source/core/index.ts#L1618,L1620

get aborted(): boolean {
    return (this[kRequest]?.destroyed ?? this.destroyed) && !(this[kOriginalResponse]?.complete);
}

or, at least, the way it's used here:
https://github.com/sindresorhus/got/blob/master/source/core/index.ts#L1059,L1062

response.once('aborted', () => {
    if (this.aborted) {
        return;
    }

[Giotino]

@szmarczak

TODO: Remove the next if when these get fixed:
https://github.com/nodejs/node/issues/32851

has been fixed in 14.3.0

[szmarczak]

But it shouldn't be breaking imo

[szmarczak]

What's the response.complete value on the aborted event (wait 0.1s before logging)?

[Giotino]

What's the response.complete value on the aborted event (wait 0.1s before logging)?

reponse.complete is false.

But there's a difference here: this[kRequest]?.destroyed

Node.JS 14.2.0: false
Node.JS 14.3.0: true (so this.aborted is true)

[darksabrefr]

I can confirm the fix is functional. Many thanks to @Giotino & @szmarczak !