AVRO-4307: [javascript] Destroy BlockDecoder on error to stop the source

[iemejia]

What is the purpose of the change

Fix a DoS-adjacent defect in the JavaScript BlockDecoder, resolving AVRO-4307.

BlockDecoder decompresses blocks asynchronously, so many block callbacks can fail for the same bad input (corrupt block, invalid sync marker, unknown codec, or the AVRO-4287 decompression size guard). The failure path emitted 'error' once per failing block with a bare emit() and never destroyed the stream. Confirmed empirically on plain main (independent of the decompression-limit work) with a 394 KB multi-block file decoded through the real createFileDecoder path using a codec that always errors:

Metric	Before	After
'error' events	947	1
bytes read after first error	65536	0
source stream destroyed (fd released)	false	true
decoder destroyed	false	true

Root cause is in BlockDecoder._createBlockCallback (bare emit, no destroy, no guard). createFileDecoder's plain .pipe() compounds it: a destination error only unpipes the source, never destroys it, so the file descriptor leaks.

Changes:

• Add an _errored flag and a guarded _onError(err) that calls destroy(err), so exactly one error surfaces and the stream unwinds. Route all fatal errors (block callback, invalid magic, unknown codec, schema parse) through it.
• Decrement _pending before the error check (previously skipped on error, which could hang the stream at finish).
• Guard _write/_writeChunk so no further blocks are processed once errored.
• Switch createFileDecoder from .pipe() to stream.pipeline() so the source file stream is destroyed and its descriptor released on error.

Verifying this change

This change added tests and can be verified as follows:

• Added surfaces a single error for many failing blocks — asserts exactly one 'error' event (no storm) and that the decoder is destroyed when every block fails.
• Added createFileDecoder tears down the source on error — asserts a single error and decoder teardown on the createFileDecoder path.
• Full JS suite passes (npm test, 385 + new tests) and jshint (npm run lint) is clean.

Documentation

• Does this pull request introduce a new feature? no
• If yes, how is the feature documented? not applicable

[Copilot]

Pull request overview

This PR addresses AVRO-4307 in the JavaScript implementation by ensuring BlockDecoder surfaces a fatal decode/decompression error only once and tears down the decoding stream so upstream sources stop reading (preventing error storms and resource leaks). It also updates createFileDecoder to use stream.pipeline() so the source file stream is destroyed on failure.

Changes:

• Add a guarded BlockDecoder._onError(err) with an _errored flag and route fatal header/block errors through destroy(err).
• Fix _pending bookkeeping on block callback errors and stop processing further blocks/writes after the decoder has errored.
• Switch createFileDecoder from .pipe() to stream.pipeline() and add tests for single-error behavior and teardown.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
lang/js/lib/files.js	Adds single-error teardown logic to BlockDecoder and uses stream.pipeline() in createFileDecoder to stop/destroy the source on failure.
lang/js/test/test_files.js	Adds regression tests for “single error” behavior and createFileDecoder error-path teardown.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

lang/js/lib/files.js:264

• invalid sync marker currently errors via cb(new Error(...)) rather than _onError(). That means _errored is never set for this fatal path, so later async block callbacks (already in-flight) can still reach _onError and potentially surface additional errors. Also, if a later write arrives after an earlier destroy, the _errored guard returns without calling cb. Route the sync-marker failure through _onError() and always invoke cb() when bailing due to _errored.

  while ((block = tryReadBlock(tap))) {
    if (this._errored) {
      return; // A prior block already failed and destroyed the stream.
    }
    if (!this._syncMarker.equals(block.sync)) {
      cb(new Error('invalid sync marker'));
      return;
    }
    this._decompress(block.data, this._createBlockCallback());

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.