NodeJs v8 stream.push() after EOF #60

Eldar-X · 2017-05-31T18:27:17Z

After upgrade to v8 i got error in console

Error: stream.push() after EOF at readableAddChunk (_stream_readable.js:227:30) at Gzip.Readable.push (_stream_readable.js:195:10) at Gzip.Transform.push (_stream_transform.js:151:32) at Zlib.callback (zlib.js:430:16)

The text was updated successfully, but these errors were encountered:

tuananh · 2017-06-02T02:05:52Z

Confirmed. I get this issue with Node v8 as well.

tuananh · 2017-06-08T12:37:22Z

Anyone taking this? Can someone point me to where I should look?

tuananh · 2017-06-08T22:23:45Z

Notice node 8.1 fixes something in zlib. Is this related?

A regression in the Zlib module that made it impossible to properly subclasses zlib.Deflate and other Zlib classes has been fixed. [6aeb555cc4] #13374.

andykant · 2017-06-09T17:10:28Z

This still occurs in Node v8.1.0.

niftylettuce · 2017-06-09T20:51:10Z

Same issue, v8.0.0

tuananh · 2017-06-13T13:53:01Z

@jonathanong could you please take a look?

jonathanong · 2017-06-13T17:59:31Z

i've seen this before. i haven't investigated it yet. it is one of the blockers for me to upgrading to node 8.

tuananh · 2017-06-20T13:07:20Z

anyone found a fix for this? also, is there an alternative middleware?

Eldar-X · 2017-06-20T16:10:07Z

In my case, i found solution by changing order of compress and conditional-get middleware

    app.use(compress({
      flush: require('zlib').Z_SYNC_FLUSH
    }))
    .use(conditional())
    .use(etag())

jonathanong · 2017-06-20T16:41:16Z

is it because you are using Z_SYNC_FLUSH? what happens if you don't use a flush parameter?

jonathanong · 2017-06-20T17:04:41Z

e50d15f seems okay to me.

anyone else have any ideas?

Eldar-X · 2017-06-20T17:37:50Z

@jonathanong without flush parameter i have same result but if koa-compress middleware over conditional-get middleware everything okay. Looks like this issue happening only when we use conditional-get middleware.

jonathanong · 2017-06-21T03:57:37Z

okay, i think the time i saw this error was with v8.0.0. i don't see it anymore with the latest node

tuananh · 2017-06-21T04:14:54Z

It seems 8.1.2 fixes it. Any idea what causes this in the first place?

pastelsky · 2017-06-21T04:30:01Z

Still experiencing this in 8.1.2. I don't have conditional-get middleware.

Eldar-X · 2017-06-21T16:38:14Z

I have same issue in 8.1.2 but with conditional-get middleware. I see this issue once more when my stream was broken.

jonathanong · 2017-06-21T17:00:10Z

can anyone make a PR with a failing test casE?

mcollina · 2017-06-22T15:32:38Z

Can you check if nodejs/node#13850 this solves the issue?

andykant · 2017-07-06T05:11:00Z

Node 8.1.3 fixes the issue.

tuananh · 2017-07-06T05:20:27Z

8.1.3 works for me as well

jonathanong · 2017-07-06T17:10:28Z

closing. thanks @mcollina !

Eldar-X · 2017-07-10T09:15:27Z

node v8.1.3 still same issue

const app = new Koa();

const conditional = require('koa-conditional-get');
const etag = require('koa-etag');
const compress = require('koa-compress');
const serveStatic = require('koa-static');

app.use(conditional())
app.use(etag())
app.use(compress())
app.use(serveStatic('public'))

app.use(async(ctx, next) => {
  ctx.type = 'text/html';
  ctx.body = `<!DOCTYPE html>
  <html lang="en">
  <head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <meta http-equiv="X-UA-Compatible" content="ie=edge">
    <title>Document</title>
    <link rel="stylesheet" href="/test.css">
  </head>
  <body>
  Response OK
  </body>
  </html>`;
})

app.listen(3000)

Don't forget to include test css with some content to public dir

mrchief · 2017-07-12T15:38:58Z

I don't use conditional get or etag and still see this issue (with node 8.1.3 and even using Z_SYNC_FLUSH). I'm using koa-webpack middleware though.

mcollina · 2017-07-12T17:39:33Z

I'll be happy to have a look, if you can post an example to reproduce the issue consistently.

Eldar-X · 2017-07-12T19:20:40Z

@mcollina you can try my example only install deps and run this app then visit page and reload it! After reloading you can see this message in your console

I am using Marko.js and i streaming response to ctx.body but if i have error in my template file i also see this error message.

mrchief · 2017-07-12T21:09:00Z

@mcollina My setup is bit complicated but @Eldar-X setup will do.

mcollina · 2017-07-13T08:10:17Z

@Eldar-X @mrchief I cannot reproduce with node 8.1.2, 8.1.3, 8.1.4 or master using your example on Mac OS X 10.11 and Chrome.

mrchief · 2017-07-13T17:09:54Z

Could the OS be a factor? I'm on Windows 7 x64. Even in my setup, I don't get it consistently. E.g., I ran my app right now and there were no errors. Then I refreshed the page and saw this error. And sometimes it happens many times within the same request.

I'll try to upload my setup tonight.

Eldar-X · 2017-07-13T17:19:43Z

@mcollina i am also on Mac Os 10.11

Eldar-X · 2017-07-13T17:25:21Z

https://github.com/Eldar-X/koa-issue my simple setup

mcollina · 2017-07-13T17:34:08Z

Confirmed, this is still happening. Quite interestly enough, this does not happen with CURL. I'll see if I can track this down (it might take a while).

It seems a different one than nodejs/node#13850.

mcollina · 2017-07-17T16:53:56Z

Now I cannot reproduce anymore. However, the same node version/code was showing the problem while I was traveling (and I could not work on it). If someone has a consistent way of reproducing this problem, please let me know.

mcollina · 2017-07-17T17:28:52Z

I can reproduce by loading the server with autocannon:

autocannon -H Accept-Encoding=gzip -c 100 -d 5 localhost:3000

If the stream is destroyed while the transform is still being applied, push() should not be called, and the internal state should be cleared. See: koajs/compress#60

mcollina · 2017-07-17T17:41:44Z

Here is the fix: nodejs/node#14330

If the stream is destroyed while the transform is still being applied, push() should not be called, and the internal state should be cleared. See: koajs/compress#60 PR-URL: #14330 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>

refack · 2017-07-19T12:54:55Z

Will you be adding a regression test for this? Then maybe apply for addition to node's CitGM

If the stream is destroyed while the transform is still being applied, push() should not be called, and the internal state should be cleared. Refs: koajs/compress#60 PR-URL: nodejs#14330 Backport-PR-URL: nodejs#14396 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Conflicts: lib/zlib.js

mcollina · 2017-07-20T22:55:37Z

This should be definitely fixed in Node 8.2.1.

refack · 2017-07-21T19:34:24Z

@jonathanong if you could write a regression test for this. It's literally keeping up at night 😨

jonathanong · 2017-07-22T18:19:38Z

@refack im not sure what the test case is...

refack · 2017-07-22T18:21:55Z

@jonathanong this might help https://github.com/nodejs/node/pull/14330/files 🤷‍♂️

dominictobias · 2018-03-14T09:08:22Z

I have it on 8.10.0 on windows

mcollina · 2018-03-14T09:30:10Z

@dominictobias can you please confirm that #60 (comment) can be used to reproduce the problem?

dominictobias · 2018-03-14T09:32:13Z

Ah don't worry, I speed read on a number of issues and this is a general problem with npm cache and windows, rather than something koa specific 😳

dandradesg · 2018-07-17T15:40:31Z

npm cache clean --force

rimiti · 2018-07-18T16:05:35Z

In my case, after making a npm cache clean --force, everything works again.

jonathanong closed this as completed Jul 6, 2017

mcollina mentioned this issue Jul 17, 2017

zlib: check if the stream is destroyed before push nodejs/node#14330

Closed

3 tasks

haoxins mentioned this issue Aug 7, 2017

stream.push() after EOF #62

Closed

NodeJs v8 stream.push() after EOF #60

NodeJs v8 stream.push() after EOF #60

Comments

Eldar-X commented May 31, 2017

tuananh commented Jun 2, 2017

tuananh commented Jun 8, 2017

tuananh commented Jun 8, 2017

andykant commented Jun 9, 2017

niftylettuce commented Jun 9, 2017

tuananh commented Jun 13, 2017

jonathanong commented Jun 13, 2017

tuananh commented Jun 20, 2017

Eldar-X commented Jun 20, 2017

jonathanong commented Jun 20, 2017

jonathanong commented Jun 20, 2017

Eldar-X commented Jun 20, 2017

jonathanong commented Jun 21, 2017

tuananh commented Jun 21, 2017

pastelsky commented Jun 21, 2017

Eldar-X commented Jun 21, 2017

jonathanong commented Jun 21, 2017

mcollina commented Jun 22, 2017

andykant commented Jul 6, 2017

tuananh commented Jul 6, 2017

jonathanong commented Jul 6, 2017

Eldar-X commented Jul 10, 2017 • edited Loading

mrchief commented Jul 12, 2017

mcollina commented Jul 12, 2017

Eldar-X commented Jul 12, 2017

mrchief commented Jul 12, 2017

mcollina commented Jul 13, 2017

mrchief commented Jul 13, 2017

Eldar-X commented Jul 13, 2017

Eldar-X commented Jul 13, 2017

mcollina commented Jul 13, 2017

mcollina commented Jul 17, 2017

mcollina commented Jul 17, 2017

mcollina commented Jul 17, 2017

refack commented Jul 19, 2017

mcollina commented Jul 20, 2017

refack commented Jul 21, 2017

jonathanong commented Jul 22, 2017

refack commented Jul 22, 2017

dominictobias commented Mar 14, 2018

mcollina commented Mar 14, 2018

dominictobias commented Mar 14, 2018 • edited Loading

dandradesg commented Jul 17, 2018

rimiti commented Jul 18, 2018 • edited Loading

Eldar-X commented Jul 10, 2017 •

edited

Loading

dominictobias commented Mar 14, 2018 •

edited

Loading

rimiti commented Jul 18, 2018 •

edited

Loading