Copilot commented on code in PR #3875:
URL: https://github.com/apache/avro/pull/3875#discussion_r3568406317
##########
lang/js/test/test_files.js:
##########
@@ -518,6 +518,52 @@ describe('files', function () {
encoder.end(1);
});
+ it('surfaces a single error for many failing blocks', function (cb) {
+ // A codec that fails on every block must not emit one 'error' per block
+ // (an error storm); only the first should surface and the stream should
be
+ // destroyed.
+ var t = createType('int');
+ var codecs = {
+ 'null': function (data, cb) { cb(new Error('ouch')); }
+ };
+ var encoder = new streams.BlockEncoder(t, {codec: 'null', blockSize: 1});
+ var decoder = new streams.BlockDecoder({codecs: codecs});
+ var errorCount = 0;
+ decoder.on('data', function () {}).on('error', function () {
errorCount++; });
+ encoder.pipe(decoder);
+ // Write many records; blockSize 1 forces many separate blocks.
+ for (var i = 0; i < 100; i++) { encoder.write(i); }
+ encoder.end();
+ setTimeout(function () {
+ assert.equal(errorCount, 1);
+ assert(decoder.destroyed);
+ cb();
+ }, 100);
Review Comment:
The test currently waits a fixed 100ms before asserting the error count.
Since the failing codec can invoke its callback synchronously, relying on
timing is both slower and can miss races. Register a `'close'` (or `'error'` +
`'close'`) handler before piping/writing and finish the test when the decoder
is actually destroyed.
##########
lang/js/test/test_files.js:
##########
@@ -518,6 +518,52 @@ describe('files', function () {
encoder.end(1);
});
+ it('surfaces a single error for many failing blocks', function (cb) {
+ // A codec that fails on every block must not emit one 'error' per block
+ // (an error storm); only the first should surface and the stream should
be
+ // destroyed.
+ var t = createType('int');
+ var codecs = {
+ 'null': function (data, cb) { cb(new Error('ouch')); }
+ };
+ var encoder = new streams.BlockEncoder(t, {codec: 'null', blockSize: 1});
+ var decoder = new streams.BlockDecoder({codecs: codecs});
+ var errorCount = 0;
+ decoder.on('data', function () {}).on('error', function () {
errorCount++; });
+ encoder.pipe(decoder);
+ // Write many records; blockSize 1 forces many separate blocks.
+ for (var i = 0; i < 100; i++) { encoder.write(i); }
+ encoder.end();
+ setTimeout(function () {
+ assert.equal(errorCount, 1);
+ assert(decoder.destroyed);
+ cb();
+ }, 100);
+ });
+
+ it('createFileDecoder tears down the source on error', function (cb) {
+ // Write a valid file, then decode it with a codec that always fails; the
+ // underlying file stream must be destroyed (descriptor released) rather
+ // than left reading a file already known to be bad.
+ var t = createType('int');
+ var filePath = tmp.fileSync().name;
+ var encoder = files.createFileEncoder(filePath, t);
+ for (var i = 0; i < 100; i++) { encoder.write(i); }
+ encoder.end();
+ encoder.getDownstream().on('finish', function () {
+ var errorCount = 0;
+ var decoder = files.createFileDecoder(filePath, {
+ codecs: { 'null': function (data, cb) { cb(new Error('ouch')); } }
+ });
+ decoder.on('data', function () {}).on('error', function () {
errorCount++; });
+ setTimeout(function () {
+ assert.equal(errorCount, 1);
+ assert(decoder.destroyed);
+ cb();
+ }, 100);
+ });
Review Comment:
This test is named/commented as if it verifies the *source* file stream is
destroyed, but it currently only asserts `decoder.destroyed` and uses a fixed
100ms timeout. You can make this deterministic and validate the actual teardown
by briefly wrapping `fs.createReadStream` to capture the ReadStream instance,
then assert `src.destroyed` when the decoder closes.
--
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]
