avsc icon indicating copy to clipboard operation
avsc copied to clipboard

Error: stream.push() after EOF

Open damion-add-on-depot opened this issue 4 years ago • 4 comments

Built a standalone browser(javascript) distribution bundle from the etc/browser folder using the following browserify command:

browserify --standalone avro avsc.js | uglify -o "dist/avsc.js"

Tested the BlockEncoder class using the function below:

function blockEncoderTest() {
    let BlockEncoder = avro.streams.BlockEncoder;
    let t = avro.Type.forSchema('int');
    let chunks = [];

    let encoder = new BlockEncoder(t, {
        omitHeader: true,
    });

    encoder.on('data', function(chunk) { 
        console.log(chunk);             
        chunks.push(chunk);
    });

    encoder.write(12);
    encoder.end();
}

Ended up with the following error:

Error: stream.push() after EOF
    at new NodeError (dist/avsc.js:8498:38)
    at readableAddChunk (dist/avsc.js:8849:60)
    at BlockEncoder.Readable.push (dist/avsc.js:8824:32)
    at BlockEncoder._read (dist/avsc.js:12778:30)
    at dist/avsc.js:12801:38
    at BlockEncoder.null [as _compress] (dist/avsc.js:12710:33)
    at BlockEncoder._flushChunk (dist/avsc.js:12761:30)
    at BlockEncoder.<anonymous> (dist/avsc.js:12697:38)
    at BlockEncoder.emit (dist/avsc.js:3318:21)
    at finishMaybe (dist/avsc.js:9986:40)

damion-add-on-depot avatar Apr 22 '21 05:04 damion-add-on-depot

I'm unable to reproduce this. Can you add more context (browser, avsc version, ...)?

mtth avatar Apr 24 '21 14:04 mtth

Its not being run 'in the browser' per say but rather via Google Apps Script which uses Chrome's V8 Javascript engine, so its functionally equivalent. I'm also using version 5.6.2 of avsc.

After a lot of debugging and trial-and-error I was able to get the library to work by modifying the _read method of the BlockEncoder class as follows:

BlockEncoder.prototype._read = function () {
    var self = this;
    var data = this._queue.pop();
    if (!data) {
        if (this._finished && !this._pending) {
            process.nextTick(function () {
                self.push(null)
            })
        } else {
            this._needPush = true
        }
        return
    }

    var state = this._readableState; 
   
    // add object count
    this.push(LONG_TYPE.toBuffer(data.count, true));
    state.ended = false;
    
    // add size in bytes
    this.push(LONG_TYPE.toBuffer(data.buf.length, true));
    state.ended = false;
    
    // add data/records
    this.push(data.buf);
    state.ended = false;
    
    // add sync token
    this.push(this._syncMarker);
    if (!this._finished) {
        data.cb()
    }
};

I basically referenced the ReadableState object and reset the state.ended property on that object after each call to push (sans the last push that adds the terminating sync token to the container buffer).

So in the context of Google App Script this works.

But I'm not really comfortable with this as its basically a hack and I have yet to pinpoint the root of the issue (the nearest I got was the realization that _readableState.ended , when true, raised the error and I worked backwards from there ).

Do you foresee any problems with this workaround?

damion-add-on-depot avatar Apr 28 '21 13:04 damion-add-on-depot

Setting state.ended might violate stream invariants; I would not recommend it. It's hard for me to say more without being able to reproduce it. My next step would be to figure out why the stream ends prematurely. Usually it happens after a push(null) - does one of these happen early?

mtth avatar Apr 29 '21 04:04 mtth

Yup,

If I recall correctly, the first push (which sets the object count) executes without error, but it sets up a state ( _readableState.ended becomes true) where subsequent calls to push() fail.

I used Google Apps Script's built-in debugger to trace the execution and I distinctly remember chunk being set to null and passed to push () at one point. Not sure why though. I'll keep digging and report back here if I figure anything out.

damion-add-on-depot avatar Apr 29 '21 13:04 damion-add-on-depot