Skip to content

fix(stream-parser): respect backpressure when importing readable streams - #724

Merged
jeswr merged 1 commit into
rdfjs:mainfrom
jeswr:codex/fix-stream-parser-import-pressure
Sep 5, 2026
Merged

fix(stream-parser): respect backpressure when importing readable streams#724
jeswr merged 1 commit into
rdfjs:mainfrom
jeswr:codex/fix-stream-parser-import-pressure

Conversation

@jeswr

@jeswr jeswr commented Sep 5, 2026

Copy link
Copy Markdown
Member

Use stream.pipe(this) when importing a readable stream so its producer pauses when the parser's buffers fill and resumes as quads are consumed. Previously import() attached a data listener and ignored write() returning false, allowing an unread parser to consume and retain the entire source.

Keep explicit source-error forwarding and the existing data/end listener fallback for RDF/JS event sources that do not provide pipe(). The return value remains the parser.

The regression test imports 30,000 one-triple chunks, checks that production pauses before exhausting the input, then consumes and verifies all 30,000 quads. It fails on the original implementation and passes with this patch. A second test covers an EventEmitter-only source.

Validation: all 6,891 tests pass with 100% coverage; ESLint, Node/browser builds, and git diff --check pass.

This fixes pre-existing behavior on main and is independent of #721 and the separate readable-buffer initialization change.

Copilot AI lite review requested due to automatic review settings September 5, 2026 15:12

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The change is small, aligns with Node stream backpressure semantics, preserves the documented fallback behavior, and is covered by targeted regression tests.

Pull request overview

This PR updates StreamParser#import() to respect Node stream backpressure when the imported source is a proper readable stream, preventing the parser from buffering the entire input when its output is not being consumed.

Changes:

  • Switch StreamParser#import() to use stream.pipe(this) when available, preserving backpressure behavior.
  • Keep explicit source error forwarding and retain the existing data/end listener fallback for non-stream event sources.
  • Add regression tests covering (1) backpressure pausing/resuming and (2) the non-pipe() EventEmitter fallback behavior.
File summaries
File Description
test/N3StreamParser-test.js Adds regression tests for backpressure behavior and for importing an EventEmitter-only source.
src/N3StreamParser.js Uses pipe() for readable streams to respect backpressure; preserves error forwarding and fallback event-based import.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@jeswr
jeswr enabled auto-merge September 5, 2026 15:42
@jeswr
jeswr disabled auto-merge September 5, 2026 18:53
@jeswr
jeswr merged commit 4788065 into rdfjs:main Sep 5, 2026
55 checks passed
@jeswr
jeswr deleted the codex/fix-stream-parser-import-pressure branch September 5, 2026 18:55
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 2.7.6 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants