Got: stream pipeline ends without an error, despite request timeout

[AshenCoon]

Describe the bug

• Node.js version: v12.16.1
• OS & version: Ubuntu 18.04.4 LTS

Actual behavior

On request timeout stream "error" event occurrs after stream "end" event. This cause pipeline to end without an error

Expected behavior

error should be passed to pipeline callback

Code to reproduce

const writeStream = fs.createWriteStream(path.resolve(__dirname, "test"));
const reqStream = got.stream(
  "<url that takes more than 1s to download>",
  {
    method: "get",
    timeout: 1000,
  }
);

reqStream.on("error", (err) => {
  console.log("reqStream.error", err.message);
});

stream.pipeline(reqStream, writeStream, (err) => {
  if (err) {
    console.log("pipeline.error", err.message);
  } else {
    console.log("pipeline.success");
  }
});

execution result:

pipeline.success
reqStream.error Timeout awaiting 'request' for 1000ms

Checklist

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

[Giotino]

1269

[AshenCoon]

Not sure if that's the same issue. Error event emits after end without using pipeline. Though in the current stable node(v13.10.1) it seems to be fixed.

[szmarczak]

Seems like a bug. Can you make a full reproducible example?

[AshenCoon]

It really is like in the head post, except for require and real url.
the only package installed is got v11.1.4.

const fs = require("fs");
const path = require("path");
const stream = require("stream");
const got = require("got");

const writeStream = fs.createWriteStream(path.resolve(__dirname, "test"));
const reqStream = got.stream("https://speed.hetzner.de/100MB.bin", {
  method: "get",
  timeout: 1000,
});

reqStream.on("error", (err) => {
  console.log("reqStream.error", err.message);
});

reqStream.on("end", () => {
  console.log("reqStream.end");
});

stream.pipeline(reqStream, writeStream, (err) => {
  if (err) {
    console.log("pipeline.error", err.message);
  } else {
    console.log("pipeline.success");
  }
});

the output is

reqStream.end
pipeline.success
reqStream.error Timeout awaiting 'request' for 1000ms

[szmarczak]

It seems to be fixed on Node.js 12.16.3 https://runkit.com/szmarczak/5ecee0fbee18e80013e53258

[AshenCoon]

Hm. I cloned your notebook and second time i ran it i got what i described
https://runkit.com/ashencoon/5ecf6eb4330279001b9a5de5
It seems like its not consistent.

[kirillgroshkov]

We're facing this too! Node 12.16.2

[Giotino]

I might have found the possible cause
writable.end([chunk[, encoding]][, callback])
The callback in Node.JS >= 14 is also used for the error event and not only for the finish event.

Node.JS 14: https://nodejs.org/api/stream.html#stream_writable_end_chunk_encoding_callback
Node.JS 12: https://nodejs.org/docs/latest-v12.x/api/stream.html#stream_writable_end_chunk_encoding_callback

[szmarczak]

I cannot reproduce the bug on Node.js > 12.18.2.

[szmarczak]

I can always reproduce this on Node.js 12.18.2:

const fs = require("fs");
const path = require("path");
const stream = require("stream");
const got = require("got");

const writeStream = fs.createWriteStream(path.resolve(__dirname, "blah"));
const reqStream = got.stream("https://speed.hetzner.de/100MB.bin", {
  method: "get",
  timeout: 1000,
  hooks: {
      beforeError: [
          async error => {
            await new Promise(resolve => setTimeout(resolve, 1000));
            return error;
          }
      ]
  }
});

reqStream.on("error", (err) => {
  console.log("reqStream.error", err.message);
});

reqStream.on("end", () => {
  console.log("reqStream.end");
});

stream.pipeline(reqStream, writeStream, (err) => {
  if (err) {
    console.log("pipeline.error", err.message);
  } else {
    console.log("pipeline.success");
  }
});

seems to be fixed for newer Node.js versions.