Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cachebusting parameter to /sockjs/info requests. #129

Closed
wants to merge 1 commit into from
Closed

Add cachebusting parameter to /sockjs/info requests. #129

wants to merge 1 commit into from

Conversation

n1mmy
Copy link

@n1mmy n1mmy commented Jul 25, 2013

Not sure if this is the best approach, but it fixes our immediate problem.

To replicate the behavior yourself, use Chrome load a SockJS app that has a relatively high latency (eg, deploy to a server far away, or use cell phone connection, or add artificial delay in your server with setTimeout), then hit reload a lot. The timing is a little finicky, you're aiming to hit reload sometime after the sockjs info request has started but before it has received a response from the server. If you hit reload at the right time, the tab is now 'poisoned' and will never establish a sockjs connection to the server. New tabs are fine. Clearing the browser cache or restarting the browser also fixes it.

This patch adds a random cachebusting url parameter to the /info request. I believe all the other requests that sockjs makes have already some sort of random string in them.

This is to work around an issue in chrome:
https://code.google.com/p/chromium/issues/detail?id=263981

It may also be useful in dealing with misbehaving proxies.
@majek
Copy link
Member

majek commented Jul 25, 2013

Ouch. Not nice, Chrome, not nice..

@emirotin
Copy link

I confirm the issue, we encountered it on Meteor project at heroku
Seems like it's exactly Chrome version 27 (and 28 now) who has it broken

AlexeyMK pushed a commit to AlexeyMK/meteor that referenced this pull request Nov 13, 2013
@brycekahle
Copy link
Contributor

@n1mmy @emirotin Do you think this is still necessary? It appears the Chrome bug has been fixed (according to the link provided in the PR)

@emirotin
Copy link

I don't know, don't work with that Meteor project anymore.

@brycekahle
Copy link
Contributor

Closing because this doesn't seem needed anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants