Protocol error on push promise #237

lewispham · 2015-12-23T13:42:59Z

I got the protocol error while I was trying to push multiple files via PUSH_PROMISE. Here is the sample code.

spdy.createServer(options, function(req, res) {
    ['/app/main.js','/app/main.css','/app/favicon-32x32.png'].forEach(function(url){
        console.log('fetching',url);
      var stream = res.push(url, {
          'content-type': `${MIME[url.substr(url.lastIndexOf('.')+1)]};charset=utf-8`,
          'content-encoding' : 'gzip',
        });
      stream.on('error', function(er) {
          throw er;
      });
      var fstream = fs.createReadStream('/home'+url);
      fstream.pipe(stream);
    });


  res.end(`
    <!DOCTYPE html>
    <html>
        <head>
            <script src="/app/main.js" async></script>
            <link rel="stylesheet" href="/app/main.css" async>
            <link rel="icon" type="image/png" href="/app/favicon-32x32.png" sizes="32x32" async>
            <meta name="viewport" content="width=device-width, initial-scale=1">
        </head>
        <body></body>
    </html>`
  );

Result:

Anything wrong with my code? Or is this a node-spdy bug?

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

indutny · 2015-12-23T15:43:52Z

@Tresdin may I ask you to provide npm ls spdy output here?

lewispham · 2015-12-23T16:02:50Z

@indutny No problem.

That's all I have.

indutny · 2015-12-23T16:04:04Z

@Tresdin thank you! May I ask you to also run npm ls spdy-transport?

lewispham · 2015-12-23T16:07:48Z

@indutny Here you are.

indutny · 2015-12-23T21:34:18Z

Thank you! Two more question to you:

What browser are you using to test it? Does it fail in other browser too?
Could you please run the same scenario with DEBUG="spdy*" environment variable and upload the output log to gist.github.com ?

Thank you again!

lewispham · 2015-12-24T03:48:13Z

@indutny

I tested on Chrome Version 47.0.2526.106 m (64-bit). Here is the result on Firefox.

2. Could you point me out the location of log file? I've tested with node /home/app/index.js DEBUG="spdy*" but nothing else happened.

indutny · 2015-12-24T04:12:38Z

@Tresdin you will need to run it like this DEBUG="spdy*" node /home/app/index.js. Thanks for testing it on Firefox.

lewispham · 2015-12-24T04:32:54Z

@indutny My output log https://gist.github.com/tresdin/908a0ebf0ea67a9c9783

lewispham · 2015-12-24T04:49:05Z

@indutny Sorry for the wrong log. Here is the one with this error. https://gist.github.com/tresdin/e557984ceba774853e3e

indutny · 2015-12-28T16:34:05Z

Thank you very much for detailed logs. However, I'm afraid I still need a bit more information than this.

May I ask you to open chrome://net-internals/#export in Chrome before testing it (please make sure that you do not have any other tabs open so that they won't leak any private data), start the server, reproduce the issue, and click "Save to file" there in chrome. You may want to send the output log by email, just in case if it contains any private data (I will make sure nothing will be exposed).

Thank you again!

indutny · 2015-12-28T16:44:48Z

Nevermind, I know what is happening. Will create a fix today.

indutny · 2015-12-28T18:24:28Z

Ah, actually I was wrong. I still need that net-internals log. Sorry!

lewispham · 2015-12-29T03:06:29Z

@indutny I've sent the log to fedor@indutny.com.

indutny · 2015-12-29T04:22:26Z

@Tresdin thank you! So the answer for our problem is:

Received duplicate pushed stream with url: https://hostname/app/favicon-32x32.png

I think if you will handle err instead of re-throwing it in error event listener - you should be able to handle it. It doesn't look like a problem on node-spdy side.

The idea behind this error message is that you actually push streams on every incoming request, and this error happens if you push the same url twice on the same TLS connection.

Thank you very much for providing the log.

lewispham · 2015-12-29T06:54:19Z

@indutny

The idea behind this error message is that you actually push streams on every incoming request

This error still stands when requests are filtered.

if(req.url === '/'){
    ['/app/main.js','/app/main.css','/app/favicon-32x32.png'].forEach(function(url){
        console.log('fetching',url);
      var stream = res.push(url, {
          'content-type': `${MIME[url.substr(url.lastIndexOf('.')+1)]};charset=utf-8`,
          'content-encoding' : 'gzip',
        });
      stream.on('error', function(er) {
          throw er;
      });
      var fstream = fs.createReadStream('/home'+url);
      fstream.pipe(stream);
    });


  res.end(`
    <!DOCTYPE html>
    <html>
        <head>
            <script src="/app/main.js" async></script>
            <link rel="stylesheet" href="/app/main.css" async>
            <link rel="icon" type="image/png" href="/app/favicon-32x32.png" sizes="32x32" async>
            <meta name="viewport" content="width=device-width, initial-scale=1">
        </head>
        <body></body>
    </html>`
  );
}

this error happens if you push the same url twice on the same TLS connection

How can url be pushed twice on the same TLS connection with my sample code? And also, what is your solution for this error?

indutny · 2015-12-29T07:44:49Z

@Tresdin the browser may cancel any PUSH stream if it thinks that it needs to do so (be it duplicate streams, or whatever). The solution is to handle error event on PUSH stream, and act accordingly.

lewispham · 2015-12-29T08:39:07Z

@indutny I just want to know the cause of the duplication. You've confirmed that this error is not from node spdy side. So which part of my code causes this error? Or is this a Chrome bug?

indutny · 2015-12-30T02:53:53Z

@Tresdin this is not a bug at all. Browser just decides that it does not want to receive any data from that PUSH stream and cancels it. This is completely legal in HTTP2

lewispham · 2015-12-30T03:49:42Z

@indutny I think it should be considered as a bug. Browsers assume that this behaviour is incorrect on server side when some duplicated streams are sent. So the problem can only be solved when browsers stop rejecting incoming pushed streams.

indutny · 2015-12-30T03:59:35Z

@Tresdin I don't get why it is a bug. Please remove throw err from your code, and it will work just fine.

indutny closed this as completed Dec 29, 2015

lewispham mentioned this issue Dec 29, 2015

Gzip compression error #233

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Protocol error on push promise #237

Protocol error on push promise #237

lewispham commented Dec 23, 2015

indutny commented Dec 23, 2015

lewispham commented Dec 23, 2015

indutny commented Dec 23, 2015

lewispham commented Dec 23, 2015

indutny commented Dec 23, 2015

lewispham commented Dec 24, 2015

indutny commented Dec 24, 2015

lewispham commented Dec 24, 2015

lewispham commented Dec 24, 2015

indutny commented Dec 28, 2015

indutny commented Dec 28, 2015

indutny commented Dec 28, 2015

lewispham commented Dec 29, 2015

indutny commented Dec 29, 2015

lewispham commented Dec 29, 2015

indutny commented Dec 29, 2015

lewispham commented Dec 29, 2015

indutny commented Dec 30, 2015

lewispham commented Dec 30, 2015

indutny commented Dec 30, 2015

Protocol error on push promise #237

Protocol error on push promise #237

Comments

lewispham commented Dec 23, 2015

indutny commented Dec 23, 2015

lewispham commented Dec 23, 2015

indutny commented Dec 23, 2015

lewispham commented Dec 23, 2015

indutny commented Dec 23, 2015

lewispham commented Dec 24, 2015

indutny commented Dec 24, 2015

lewispham commented Dec 24, 2015

lewispham commented Dec 24, 2015

indutny commented Dec 28, 2015

indutny commented Dec 28, 2015

indutny commented Dec 28, 2015

lewispham commented Dec 29, 2015

indutny commented Dec 29, 2015

lewispham commented Dec 29, 2015

indutny commented Dec 29, 2015

lewispham commented Dec 29, 2015

indutny commented Dec 30, 2015

lewispham commented Dec 30, 2015

indutny commented Dec 30, 2015