stream: handling invalid stream arguments in `stream.pipeline()`
Nobody has claimed this yet.
- Dominant language
- JavaScript
- Stars
- 122k
- Forks
- 37.3k
- Avg merge
- 4d 2h
- Merged PRs (30d)
- 283
Description
A few "what goes where"-related inconsistencies with stream.pipeline(). Validation of stream objects is very hit-and-miss:
| Passing this: | as this: | should do this: | and does this: |
|---|---|---|---|
| Non-readable Node stream | source | Should be rejected. | :x: Passes validation. Gets passed to Duplex.from(), resulting in a write-only Duplex. The pipeline will never receive any data. |
| transform | Should be rejected. | :heavy_check_mark: Fails validation. | |
| destination | Should be accepted. | :heavy_check_mark: Passes validation. | |
| Non-writable Node stream | source | Should be accepted. | :heavy_check_mark: Passes validation. |
| transform | Should be rejected. | :x: Passes validation. Fails asynchronously at runtime: when the previous stream in the pipeline emits data, raises a TypeError due to attempting to call missing Writable methods. | |
| destination | Should be rejected. | :x: Same as above. | |
ReadableStream |
source | Should be accepted. | :heavy_check_mark: Passes validation. |
| transform | Should be rejected. | :heavy_check_mark: Fails validation. | |
| destination | Should be rejected. | :x: Passes validation. Fails asynchronously at runtime: raises a TypeError due to attempting to call the missing getWriter() method. |
|
TransformStream |
source | Undocumented, but should be accepted. | :heavy_check_mark: Passes validation. |
| transform | Should be accepted. | :heavy_check_mark: Passes validation. | |
| destination | Undocumented, but should be accepted. | :heavy_check_mark: Passes validation. | |
WritableStream |
source | Should be rejected. | :x: Passes validation. Gets passed to Duplex.from(), resulting in a write-only Duplex. The pipeline will never receive any data. |
| transform | Should be rejected. | :heavy_check_mark: Fails validation. | |
| destination | Should be accepted. | :heavy_check_mark: Passes validation. |
These should probably be validated consistently.
Other observations:
- The docs should specify that
TransformStreams are valid source and destination streams, as well as valid return values. - The transform validation error message doesn't specify which parameter failed validation, which is a pain in terms of debugging. Its counterparts in
stream.compose()pass the parameter name to the error constructor asstreams[n], which would be a fairly straightforward improvement.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the linked validation paths in lib/internal/streams/pipeline.js, especially the source, transform, and destination handling around lines 306-308, 376, and 391-395. Compare the parameter naming used by stream.compose() and inspect the related stream documentation. Done means invalid arguments are rejected consistently, validation errors identify the failing parameter, and TransformStream support is documented as described.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, node.js
- Domain
- backend, documentation
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100