mafintosh / mafintosh/pumpify

fails test with latest end-of-stream

Open
#13 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
JavaScript
Stars
252
Forks
14
PR merge metrics
No merged PRs in 30d

Description

I don't exactly understand how https://github.com/mafintosh/end-of-stream/commit/c02340731e5f949ad1097cb089a84f857f0b2623 can cause the failure. But the failure seems related to two things:

- error/close is emitted asynchronously from destroy which causes unpipe to be delayed resulting in a write after destroy (https://github.com/nodejs/node/issues/29790)
- error can be emitted twice (this I believe has been fixed in Node master so it might be worth to just wait for a stream-readable update). The second error should just be swallowed. However, the problem here is that the write after destroy error might be preceded by the actual error (see comment in https://github.com/nodejs/node/issues/29790)

Ref: https://github.com/nodejs/node/pull/29504

```js
# preserves error again
ok 19 should be truthy
ok 20 does not close with premature close
ok 21 should be truthy
ok 22 does not close with premature close
not ok 23 .end() called twice
---
operator: fail
at: Pumpify. (/Users/ronagy/GitHub/public/pumpify/test.js:210:9)
stack: |-
Error: .end() called twice
at Test.assert [as _assert] (/Users/ronagy/GitHub/public/pumpify/node_modules/tape/lib/test.js:225:54)
at Test.bound [as _assert] (/Users/ronagy/GitHub/public/pumpify/node_modules/tape/lib/test.js:77:32)
at Test.fail (/Users/ronagy/GitHub/public/pumpify/node_modules/tape/lib/test.js:318:10)
at Test.bound [as fail] (/Users/ronagy/GitHub/public/pumpify/node_modules/tape/lib/test.js:77:32)
at Test.end (/Users/ronagy/GitHub/public/pumpify/node_modules/tape/lib/test.js:151:14)
at Test.bound [as end] (/Users/ronagy/GitHub/public/pumpify/node_modules/tape/lib/test.js:77:32)
at Pumpify. (/Users/ronagy/GitHub/public/pumpify/test.js:210:9)
at Pumpify.emit (events.js:209:13)
at Pumpify.Duplexify._destroy (/Users/ronagy/GitHub/public/pumpify/node_modules/duplexify/index.js:195:15)
at /Users/ronagy/GitHub/public/pumpify/node_modules/duplexify/index.js:185:10
...
```

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 with test.js around line 210 and reproduce the failure against the latest end-of-stream and Node behavior described in the issue. Read the related duplexify/index.js destroy path and the linked Node issue and pull request. Done means the failing test no longer reports the duplicate .end() failure while preserving the relevant stream error behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, node.js
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.