iemejia commented on code in PR #3852:
URL: https://github.com/apache/avro/pull/3852#discussion_r3567623299
##########
lang/js/lib/files.js:
##########
@@ -247,11 +315,21 @@ BlockDecoder.prototype._createBlockCallback = function ()
{
this._pending++;
return function (err, data) {
+ // Always account for this block's completion, even on error, so a failed
+ // decompression (e.g. the size safeguard firing) cannot leave `_pending`
+ // permanently above zero and hang the stream at finish.
+ self._pending--;
if (err) {
self.emit('error', err);
+ // If the writable side already finished and this was the last pending
+ // block, end the readable side too so a consumer waiting on the queue is
+ // not left hanging after the error.
+ if (self._needPush && self._finished && !self._pending) {
+ self._needPush = false;
+ self.push(null);
+ }
return;
}
Review Comment:
The decoder's error contract is to emit 'error' (consumers listen for it),
which the size-safeguard path does. Switching to `destroy(err)` changes the
stream lifecycle (emits 'close', marks destroyed) and, for a piped source,
cleaning up the upstream ReadStream on a destination error is really the
caller's responsibility — `stream.pipeline()` does exactly that and is the
recommended way to wire these up. Changing the decoder's teardown semantics
late in this PR risks affecting existing consumers, so I'd prefer to keep the
emit-'error' contract here; a destroy()/pipeline-based cleanup can be a focused
follow-up if desired.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
