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

Stop sockjs from multiplying on reconnect #888

Merged
merged 2 commits into from May 11, 2015

Conversation

Projects
None yet
3 participants
@markwal
Copy link
Collaborator

commented May 11, 2015

Have you ever noticed when developing that every time you stop and start
the server, the terminal window gets an extra duplicate line for every
reconnect attempt? Well, it's because (I think) "delete" in javascript
just removes the indicated name from the namespace, it doesn't actually
free up an object. Those zombie objects are still there and wake up (for
some transports) on reconnect. Might be different in SockJS v1 or later.

Anyway, this fixes it for me in Chrome.

Stop sockjs from multiplying on reconnect
Have you ever noticed when developing that every time you stop and start
the server, the terminal window gets an extra duplicate line for every
reconnect attempt?  Well, it's because (I think) "delete" in javascript
just removes the indicated name from the namespace, it doesn't actually
free up an object. Those zombie objects are still there and wake up (for
some transports) on reconnect. Might be different in SockJS v1 or later.
@foosel

This comment has been minimized.

Magic number? Where does that come from?

This comment has been minimized.

Copy link
Owner Author

replied May 11, 2015

Sockjs sources. They used a magic number too, no define or docs that I could find. It means from a direct call to close in v0.3.4.

Probably we should define our own. It'll give back whatever we pass to close. But still weird since sockjs could still stomp on it with the same value used internally.

This comment has been minimized.

Copy link

replied May 11, 2015

Let me just say "ewww"

@nophead

This comment has been minimized.

Copy link
Contributor

commented May 11, 2015

Yes I have seen the terminal lines get duplicated, then triplicated, etc.
Not developing though, just using OctopPrint.

On 11 May 2015 at 03:03, Mark Walker notifications@github.com wrote:

Have you ever noticed when developing that every time you stop and start
the server, the terminal window gets an extra duplicate line for every
reconnect attempt? Well, it's because (I think) "delete" in javascript
just removes the indicated name from the namespace, it doesn't actually
free up an object. Those zombie objects are still there and wake up (for
some transports) on reconnect. Might be different in SockJS v1 or later.

Anyway, this fixes it for me in Chrome.

You can view, comment on, or merge this pull request online at:

#888
Commit Summary

  • Stop sockjs from multiplying on reconnect

File Changes

Patch Links:

Reply to this email directly or view it on GitHub
#888.

@markwal

This comment has been minimized.

Copy link
Owner Author

commented on 91bdffa May 11, 2015

OK, https://developer.mozilla.org/en-US/docs/Web/API/CloseEvent defines it as CLOSE_NORMAL, but sockjs just says 1000 in their source. But also, they reserve 4000-4999 for application defined. So which way do you want to go? Use 1000 because that's the MDN defined CLOSE_NORMAL? We could define it here since sockjs doesn't yet. Or make up our own? var OCTOPRINT_CLOSE_NORMAL = 4000

Magic CLOSE_NORMAL
See https://developer.mozilla.org/en-US/docs/Web/API/CloseEvent
WebSocket defines 1000 as CLOSE_NORMAL, SockJS uses it without
a name and doesn't provide one for us, so for now we define our own

foosel added a commit that referenced this pull request May 11, 2015

@foosel foosel merged commit c8b8bcb into foosel:devel May 11, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@markwal markwal deleted the markwal:sockjslikebunnies branch May 11, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.