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

Problem after updating from 2.0.2 to 2.0.3 with middleware #2971

Closed
ghost opened this issue Jun 13, 2017 · 12 comments
Closed

Problem after updating from 2.0.2 to 2.0.3 with middleware #2971

ghost opened this issue Jun 13, 2017 · 12 comments

Comments

@ghost
Copy link

ghost commented Jun 13, 2017

This error occurred the first time after updating from 2.0.2 to 2.0.3 - I really can't figure it out right now.

/data/project/app/node_modules/on-headers/index.js:25
	throw new TypeError('argument res is required')
	^

TypeError: argument res is required
	at onHeaders (/data/project/app/node_modules/on-headers/index.js:25:11)
	at session (/data/project/app/node_modules/express-session/index.js:219:5)
	at Array.<anonymous> (/data/project/app/modules/webserver.js:41:5)
	at run (/data/project/app/node_modules/socket.io/lib/namespace.js:123:11)
	at Namespace.run (/data/project/app/node_modules/socket.io/lib/namespace.js:135:3)
	at Namespace.add (/data/project/app/node_modules/socket.io/lib/namespace.js:163:8)
	at Client.connect (/data/project/app/node_modules/socket.io/lib/client.js:76:20)
	at Server.onconnection (/data/project/app/node_modules/socket.io/lib/index.js:398:10)
	at emitOne (events.js:115:13)
	at Server.emit (events.js:210:7)
error: Forever detected script exited with code: 1
error: Script restart attempt #63
const session = require('express-session');
[..]
const httpsServer = https.createServer(credentials, app).listen(config.server.port);
const io = require('socket.io')(httpsServer);

var sessionMiddleware = session({
    store: new RedisStore({}),
    secret: 'MYSECRET',
    resave: false,
    saveUninitialized: true
});

io.use(function (socket, next) {
    sessionMiddleware(socket.request, socket.request.res, next); //(/data/project/app/modules/webserver.js:41:5)
});
@darrachequesne
Copy link
Member

darrachequesne commented Jun 13, 2017

@gindu as usual, could you please provide a way to reproduce?

Fiddle here: https://github.com/darrachequesne/socket.io-fiddle/tree/issues-2971

@ghost
Copy link
Author

ghost commented Jun 15, 2017

Only happening once (3hours after updating to 2.0.3 on live site) - reverting directly to 2.0.2.

On dev site i can't even reproduce it. So it will be hard to do this in a fiddle.

As of the error the "socket.request.res" was probably undefined/null ?

io.use(function (socket, next) {
    if(!socket) {
        console.log("#1: it happend again! " + util.inspect(socket));
        return next();
    }

    if(!socket && !socket.request) {
        console.log("#2: it happend again! " + util.inspect(socket.request));
        return next();
    }
    if(!socket && !socket.request && !socket.request.res) {

        console.log("#3: it happend again! " + util.inspect(socket.request.res));
        return next();
    }


    sessionMiddleware(socket.request, socket.request.res, next);
});

would this work without crashing the live site?

@ashnamuh
Copy link

ashnamuh commented Jul 6, 2017

I have a same issue

@ToddAlvord
Copy link

I found that this works.
sessionMiddleware( socket.request, {}, next );

You will have access to your session via socket.request.session. The res part is used to notify the session handler to check if it should save the session data or not after a request has completed. I never could get session data to save automatically with socket.io, even in v1. If you need to make changes to your session in a socket.io connection you can use socket.request.session.save(). Basically remove the res part as it wasn't being used anyways by socket.io. At least not in a way that seems to have any affect.

@ghost
Copy link
Author

ghost commented Aug 30, 2017

I found the issue for the crash. On polling socket.request.res is find. On Websocket it's undefined.

I have already swapped to express-socket.io-session module. This working without any issue

@evancarter-iex
Copy link

@darrachequesne Any possibility on getting a fix on this? We are seeing the same issue

@yaneony
Copy link

yaneony commented Feb 5, 2018

2.0.4 same issue.

@knoxcard
Copy link

@ToddAlvord - Todd, you're a lifesaver for that one!

@akontsevich
Copy link

akontsevich commented Dec 5, 2018

Thanks, @ghost! Think we may consider to use express-socket.io-session module as an option. sessionMiddleware worked fine for me

io.use(function(socket, next) {
    sessionMiddleware(socket.request, socket.request.res, next);
});

with web socket.io-client <=> socket.io node.js server until I added socket.io-client-cpp application to the chain - crashed on socket.io-client-cpp connection with above error:

TypeError: argument res is required

@akontsevich
Copy link

@ToddAlvord thanks for the solution! Think it could be improved a little:

io.use(function(socket, next) {
    sessionMiddleware(socket.request, socket.request.res || {}, next);
});

@dimified
Copy link

Thanks! Works for me.

@darrachequesne
Copy link
Member

For future readers, the documentation was added here: https://socket.io/docs/v3/middlewares/#Compatibility-with-Express-middleware

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

No branches or pull requests

8 participants