mafintosh / mafintosh/duplexify

Unhandled 'error' event

Open
#39 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
JavaScript
Stars
191
Forks
35
PR merge metrics
No merged PRs in 30d

Description

The reason for posting this issue is that It looks like google storage fails and calls this `.destroy(...)` which throws an unhandled error. I created an [issue in the google storage repo](https://github.com/googleapis/nodejs-storage/issues/1623).

We were able to reproduce the unhandled error in a test script. We created a simple stream with `stream events`, `pumpify`, and `duplexify` and ran `.destroy(error)` which threw an unhandled error.

Kinda new to js, node and streams so I don't really understand anything that happens here but tried to debug a bit. When logging inside `_destory( ... )` and `destroy( ... )` it looks like both of them are called several times, and the internal one is actually called after "finally" has been logged. Does this make sense? Why does this crash?

### Script to reproduce error
```
import * as streamEvents from "stream-events";
const pumpify = require('pumpify');
import {Duplex, PassThrough} from 'stream';
const hashStreamValidation = require('hash-stream-validation');
const duplexify: any = require('duplexify');

class RequestError extends Error {
code?: string;
errors?: Error[];
}

function test() {
let crc32c = true;
let md5 = false;

const validateStream = hashStreamValidation({
crc32c,
md5,
});

const fileWriteStream = duplexify();
const stream = streamEvents(
pumpify([
new PassThrough(),
validateStream,
fileWriteStream,
])
) as Duplex;

let code;
let message;

code = 'FILE_NO_UPLOAD';
message = 'File upload failed.';
const error = new RequestError(message);
error.code = code;
error.errors = [];
try {
fileWriteStream.destroy(error); <-- Crashes
// fileWriteStream.destroy(); <-- Good
} catch (err) {
console.log("fileWriteStream: ", err)
}
stream.uncork();
}

try {
test();
} catch (err) {
console.log("test: ", err);
} finally {
console.log("finally");
}

```

### Environment
MacOS: 11.6
Duplexify: 4.1.2
Pumpify: 2.0.1
Hash-stream-validation: 0.2.4
Stream events: 1.0.5
Npm: 6.14.22
Node: 12.22.1

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by running the inline reproduction script with Node 12.22.1 and inspect duplexify's destroy(error) path, comparing it with destroy() and the surrounding pumpify/stream-events chain. Done means the reported destroy(error) case no longer produces an unhandled "error" event, with behavior checked against the supplied reproduction.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, nodejs
Domain
backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.