Skip to content
This repository has been archived by the owner on Aug 30, 2021. It is now read-only.

Fixed bug with Socket IO session #812

Merged
merged 1 commit into from
Aug 17, 2015

Conversation

mleanos
Copy link
Member

@mleanos mleanos commented Aug 16, 2015

New enhancements to the Express session, added "sessionId" as the new
default session key. Previously, the express session was using the
default "connect.sid" key. This caused the Socket configuration to be
unable to find the session id, thus causing issues with the Socket. One
such issue, was that the same Socket would be used for all users
connected to the server.

This also adds a bit more error handling of the Socket server
configuration.

@lirantal lirantal added this to the 0.4.x milestone Aug 16, 2015
@lirantal lirantal self-assigned this Aug 16, 2015
@lirantal
Copy link
Member

LGTM

@mleanos mleanos force-pushed the bug/socket-session-key branch from f7771bb to 42e83e8 Compare August 16, 2015 08:05
@mleanos
Copy link
Member Author

mleanos commented Aug 16, 2015

Re-worked the error handling. Using the return next() pattern for handling the errors. This makes the code cleaner, and easier to read.

@mleanos mleanos force-pushed the bug/socket-session-key branch from 42e83e8 to 8675841 Compare August 16, 2015 21:44
@mleanos
Copy link
Member Author

mleanos commented Aug 16, 2015

Attempted to fix the spacing issues.. I removed automatic indentation on new lines. @rhutchison How does it look to you now?

New enhancements to the Express session, added "sessionId" as the new
default session key. Previously, the express session was using the
default "connect.sid" key. This caused the Socket configuration to be
unable to find the session id, thus causing issues with the Socket. One
suck issue, was that the same Socket would be used for all users
connected to the server.

This also adds a bit more error handling of the Socket server
configuration. Using the `return next()` pattern.
@mleanos mleanos force-pushed the bug/socket-session-key branch from 8675841 to 792488e Compare August 16, 2015 22:09
@codydaig
Copy link
Member

LGTM

Tested, and verified that it works.

@rhutchison
Copy link
Contributor

LGTM - thanks

@lirantal
Copy link
Member

Thanks @mleanos and @rhutchison @codydaig for reviewing!

lirantal added a commit that referenced this pull request Aug 17, 2015
@lirantal lirantal merged commit 5b78706 into meanjs:master Aug 17, 2015
@mleanos mleanos deleted the bug/socket-session-key branch October 1, 2015 03:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants