search

spdy-http2 / node-spdy

SPDY server on Node.js
2.81k stars 196 forks source link

Firefox push issue #160

Closed guybedford closed 8 years ago

guybedford commented 10 years ago

I have the following loading process:

  https://jspm.io/system.js -> https://jspm.io/system@0.5.6.js (301 redirect)
  https://system@0.5.6.js SPDY pushes https://jspm.io/es6-module-loader@0.5.4.js

This works fine in Chrome, but on Firefox, sporadically the request will stall and never end (keeps spinning).

This looks quite similar to the issue in https://github.com/indutny/node-spdy/issues/158

Any ideas appreciated. Edge cases like this kill the workflow quite badly.

guybedford commented 10 years ago

Any suggestions for listeners I could add to the example in https://github.com/indutny/node-spdy/issues/158 for this sort of thing would really help too to debug.... it's just I have no idea how to start to diagnose this sort of thing.

Update - I can confirm that no stream error is being thrown in the stalling case.

guybedford commented 10 years ago

Do we think this is related to #158? For now I have disabled SPDY push entirely due to these Firefox issues.

indutny commented 10 years ago

There seems to be a problem with unconsumed PUSHes implementation in at least Chrome, I'll look into Firefox somewhere later after figuring out Chromium issues.

guybedford commented 10 years ago

Thanks, really appreciate your help. In both cases it is a push non-connection-closing error which seems very similar to me. The Chrome issue I haven't seen in production, while the Firefox issue is appearing consistently in production when I do a 301 redirect and push its redirect file at the same time, so that's why I've had to disable SPDY push for now.

indutny commented 10 years ago 
As Will said, server pushes currently reside with the session they were pushed over,
and pushed data can only be 'consumed' by a matching client request which claims
it. If no matching client requests is forthcoming (which is the case in your test server),
the data is never consumed, and corresponding window updates for the session and
stream aren't sent. Therefor push is only useful in Chromium right now if there's good
reason to expect a matching request will claim the stream, and as things stand today
your test case is working as intended.
indutny commented 10 years ago

@guybedford that's reply from Chromium team... If you have something to say - you better weigh in ;)

guybedford commented 10 years ago

@indutny does this not just need a timeout then in the server?

guybedford commented 10 years ago

If this is a window edge case, does this mean that the server just needs to avoid pushing beyond the window limit until the PUSH stream is confirmed?

Sorry I'm probably showing my ignorance more than anything else, I have a very basic understanding of this stuff.

It's just about coming up with a way to constrain this edge case though.

indutny commented 10 years ago

Ah, the thing is that the page that you are giving to the browser should always request PUSH stream.

guybedford commented 10 years ago

Is the RESET not enough information to close the stream though?

indutny commented 10 years ago

It is the first PUSH stream that gets the window filled and it is not RESETed.

guybedford commented 10 years ago

Ahh right. So could the push stream be throttled not to overflow the window until there is some feedback from the client?

indutny commented 10 years ago

Nope :) There is no way to figure this out on server, I'm currently writing an email to spdy guys to figure it out.

guybedford commented 10 years ago

Ok. It seems I really did find an edge case! I just want to reiterate, I have not seen this in production... and if I reduce the PUSH stream from 50MB to 10MB it works fine.

Perhaps the simple throttling is simply not to push a file over 10MB?

I can live with that honestly. Apologies if I've opened up a can of worms here!

indutny commented 10 years ago

Hahah, so the problem here is that your server sends some data that browser is never reclaiming. Eventually it will lead to the stalled connection.

guybedford commented 10 years ago

But isn't that right, if I'm effectively overloading the connection? Makes sense for security surely?

indutny commented 10 years ago

This is right, but implemented not really correctly. I think for now it would be better for you to just disable pushes, we'll certainly figure it out in next Chrome releases.

indutny commented 10 years ago

And perhaps get to Firefox after that.

guybedford commented 10 years ago

Ok, so I have now discovered that both SPDY push and SPDY hint are broken within the same week! Well at least there is progress :)

Multiplexing is all I need though, so I'm glad that is stable at least!