version 6.6.0 not working with socket.io

[joacub]

latest version is forcing alien mode and i dont know if this is the cause or not, but socket.io is not working anymore due this error:

Error: Using uWS.HttpRequest / uWS.Http3Request past its request handler return is forbidden (it is stack allocated).

i really didnt dig on this much but seems latest version of hyper-express was working im assuming something introduced in the update is crashing or not working normally.

UPDATE:

I tested without forcing the alien mode and it was working properly, so the issue is when is with alien mode.

is there anything that may not work as before that is causing this or is this a bug in the uWs itself ?

[kartikk221]

Hey, that's odd, all of the tests passed with alien mode on both linux and darwin based systems. Could you potentially provide a code snippet which could be causing this and also any stack traces. The error you mention should not occur at all and from what you mention the culprit may be alien mode from uWS.

[kartikk221]

Also, is your socket.io route potentially using /:id/* dynamic routes like this where you have wildcards in the path?

[joacub]

Also, is your socket.io route potentially using /:id/* dynamic routes like this where you have wildcards in the path?

I may have wildcards in the paths but I’m using this internally not through socket.io. We are only hearing sockets at /ws path and that’s it. The route are internally handled by other router system not attached to. But this also is working without the alien mode why the alien mode is causing this.

about the code snippet, just try to use a socket.io attach mode and you will see, when socket io does the upgrade this fails, only on alien mode.

this in the startup of the application so I don’t think is in any thing in the app itself.

[kartikk221]

Just saw your issue at https://github.com/uNetworking/uWebSockets.js/issues/907

So the Websocket upgrade fails? If so that is a uWebsockets.js bug because the uWS.HttpResponse object should not be deallocated and uWS.HttpResponse.upgrade() should be callable at any time.

[joacub]

Just saw your issue at uNetworking/uWebSockets.js#907

So the Websocket upgrade fails? If so that is a uWebsockets.js bug because the uWS.HttpResponse object should not be deallocated and uWS.HttpResponse.upgrade() should be callable at any time.

Yes that’s what I thought. But since I’m not an expert in uws I was asking without pointing to this as an issue. I don’t see any thing wrong in socket.io and also the example they have (uws) is almost the same. But yeah seems like the alien mode does not allowing upgrade.

let’s wait for what they say. If you see something else let me know. I already investigate and remove all routes and almost all the code I have just to check if is something in my end and the error keeps happening so almost 100% sure is an introduced bug or similar.

[joacub]

Btw, why the library is forcing to use alien mode ???

[kartikk221]

Hey, all of the library tests passed with alien mode on both Linux and Darwin (only systems enabled). HyperExpress does not do any async processing with the uWebsockets.js request object so seemed like a relatively safe thing to force for performance. However, it seems there are some bugs in your case thus I've added a flag to disable alien mode If it causes problems in the latest v6.6.1.

I am honestly not sure why in your case, upgrading throws an error because the tests for the Websocket component run fine on Linux and MacOS.

[joacub]

Did you try to run an upgrade httprequest to ws?

[kartikk221]

The HyperExpress test suite contains extensive tests for all components used internally. Here are one of the ws tests: https://github.com/kartikk221/hyper-express/blob/master/tests/components/ws/Websocket.js

You're more than welcome to clone this repository on your machine, clone all submodules / install npm modules and then run the test suite with npm run test to see if its a problem isolated to your machine.

[joacub]

I saw your comment in the issue in uws. Just to comment on that that the error is in the start of the app so there is no stress or something similar happening.

[joacub]

The HyperExpress test suite contains extensive tests for all components used internally. Here are one of the ws tests: https://github.com/kartikk221/hyper-express/blob/master/tests/components/ws/Websocket.js

You're more than welcome to clone this repository on your machine, clone all submodules / install npm modules and then run the test suite with npm run test to see if its a problem isolated to your machine.

I have a m1 Mac so the test should the same. I’m testing this in a docker image so I don’t think is related to a hosted os.

[kartikk221]

One thing I can try tomorrow is adding some delay before upgrading the response to a websocket to see if that causes a problem. I think all of the tests almost instantly upgrade the request to a websocket while in your case there me some async work happening before the upgrading. Regardless, I'll let you know If I find something. In the meantime, you can upgrade to v6.6.1 continue usage with alien mode disabled.

[joacub]

One thing I can try tomorrow is adding some delay before upgrading the response to a websocket to see if that causes a problem. I think all of the tests almost instantly upgrade the request to a websocket while in your case there me some async work happening before the upgrading. Regardless, I'll let you know If I find something. In the meantime, you can upgrade to v6.6.1 continue usage with alien mode disabled.

That’s may be the issue in the tests. One question sir. I see that your upgrade method do not pass the user parameters to uwebsocket so this changes what for example socket.up is sending as a parameters to uws. Is that for some reason is that right ?

Socket.up send this to in theory uws:

res.upgrade(
        {
          transport,
        },
        req.getHeader("sec-websocket-key"),
        req.getHeader("sec-websocket-protocol"),
        req.getHeader("sec-websocket-extensions"),
        context
);

but hyper-express upgrade method change the parameters sending to uws. So I’m theory there is a mismatch in what socket.io send and should used later. I don’t dig in this but maybe you later in the better you send back the user context as is. Let me know about this cause I’m confuse.

btw thanks for the project i really like it.

[kartikk221]

Wait I have a question. Why do you do req.getHeader(name) and not req.headers[name] to read the headers? Also, could you provide more of the code snippet or is that from the socket.io library source code?

[joacub]

Wait I have a question. Why do you do req.getHeader(name) and not req.headers[name] to read the headers? Also, could you provide more of the code snippet or is that from the socket.io library source code?

You can check the code here.

https://github.com/socketio/engine.io/blob/main/lib/userver.ts

[joacub]

Check also the example of the uwebsocket.js repo:

https://github.com/uNetworking/uWebSockets.js/blob/master/examples/Upgrade.js

they use the getter instead accesing the var

[joacub]

One thing I can try tomorrow is adding some delay before upgrading the response to a websocket to see if that causes a problem. I think all of the tests almost instantly upgrade the request to a websocket while in your case there me some async work happening before the upgrading. Regardless, I'll let you know If I find something. In the meantime, you can upgrade to v6.6.1 continue usage with alien mode disabled.

That’s may be the issue in the tests. One question sir. I see that your upgrade method do not pass the user parameters to uwebsocket so this changes what for example socket.up is sending as a parameters to uws. Is that for some reason is that right ?

Socket.up send this to in theory uws:

res.upgrade(
        {
          transport,
        },
        req.getHeader("sec-websocket-key"),
        req.getHeader("sec-websocket-protocol"),
        req.getHeader("sec-websocket-extensions"),
        context
);

but hyper-express upgrade method change the parameters sending to uws. So I’m theory there is a mismatch in what socket.io send and should used later. I don’t dig in this but maybe you later in the better you send back the user context as is. Let me know about this cause I’m confuse.

btw thanks for the project i really like it.

about this, do you have any coment ?

[kartikk221]

Hey, just added a test for no delay and some delay aka. asynchronous upgrading and the tests ran fine on both Windows and Ubuntu (Linux) so at this point I am lost as to why you are encountering that violation from uWebsockets.js. You would have to provide more code to give more context as both sync and async upgrades work fine with HyperExpress for websockets.

[joacub]

interesting, ill be doing that, to see where is the breaking. did you read what the autor of uws said about the code is broken ?

[kartikk221]

Yeah, I provided him with more context but after the above tests, I really do not think this is a hyper-express bug or even a bug with Alien mode because then the tests would not run on linux where Alien mode is enabled in my case.

I'll close this issue for now but If you find out this is something that can be mitigated by HyperExpress feel free to re-open. I will also be disabling Alien mode in future updates just to be on the safe side.

[joacub]

One thing I can try tomorrow is adding some delay before upgrading the response to a websocket to see if that causes a problem. I think all of the tests almost instantly upgrade the request to a websocket while in your case there me some async work happening before the upgrading. Regardless, I'll let you know If I find something. In the meantime, you can upgrade to v6.6.1 continue usage with alien mode disabled.

That’s may be the issue in the tests. One question sir. I see that your upgrade method do not pass the user parameters to uwebsocket so this changes what for example socket.up is sending as a parameters to uws. Is that for some reason is that right ? Socket.up send this to in theory uws:

res.upgrade(
        {
          transport,
        },
        req.getHeader("sec-websocket-key"),
        req.getHeader("sec-websocket-protocol"),
        req.getHeader("sec-websocket-extensions"),
        context
);

but hyper-express upgrade method change the parameters sending to uws. So I’m theory there is a mismatch in what socket.io send and should used later. I don’t dig in this but maybe you later in the better you send back the user context as is. Let me know about this cause I’m confuse. btw thanks for the project i really like it.

about this, do you have any coment ?

what about this, the context is not the same as the user send, you are changing them, why ?

[kartikk221]

I think the wording is confusing you here. If you look at the signature for the uWS.HttpResponse.upgrade() merthod here: https://unetworking.github.io/uWebSockets.js/generated/interfaces/HttpResponse.html#upgrade you will notice that the parameters are the following in order:

1. UserData which can be a object with any keys or values that will be available on the uWS.WebSocket object in the ws route. 2, 3, 4. These are the SEC websocket key, protocol and extensions headers.
2. This is the us_socket_context_t aka. the socket reference provided by uWebsockets.js in the upgrade callback which HyperExpress provides as upgrade_socket.

So the context that HyperExpress sets in the place of the transport is just some arbitary Object of keys and values the user can provide.

For example:

response.upgrade({
    someKey: 'someValue',
    someKey2: 'someValue2'
});

will allow you to then do:

console.log(ws.context.someKey) // -> someValue
console.log(ws.context.someKey2) // -> someValue2

[joacub]

I think the wording is confusing you here. If you look at the signature for the uWS.HttpResponse.upgrade() merthod here: https://unetworking.github.io/uWebSockets.js/generated/interfaces/HttpResponse.html#upgrade you will notice that the parameters are the following in order:

1. UserData which can be a object with any keys or values that will be available on the uWS.WebSocket object in the ws route.
2. 3 and 4 These are the SEC websocket key, protocol and extensions headers.
3. This is the us_socket_context_t aka. the socket reference provided by uWebsockets.js in the upgrade callback which HyperExpress provides as upgrade_socket.

So the context that HyperExpress sets in the place of the transport is just some arbitary Object of keys and values the user can provide.

For example:

response.upgrade({
    someKey: 'someValue',
    someKey2: 'someValue2'
});

will allow you to then do:

console.log(ws.context.someKey) // -> someValue
console.log(ws.context.someKey2) // -> someValue2

Yeah that’s the problem your are forcing us to use your context key instead of passing directly to uws, see the upgrade method is already sending that how we want to get this back. We don’t want and we don’t know other libraries will know you are doing that, you should transparent in that case and just send this straight to uws. Basically you are changing the way this work.

[joacub]

can you expose the raw uws server, so we can attack this directly, socket.io has their integration directly with uws so is not needed to wrap any layer for compatibility for them.

[joacub]

never mind, i saw you are exposing already.

[kartikk221]

Yeah that’s the problem your are forcing us to use your context key instead of passing directly to uws, see the upgrade method is already sending that how we want to get this back. We don’t want and we don’t know other libraries will know you are doing that, you should transparent in that case and just send this straight to uws. Basically you are changing the way this work.

The reason why the user's keys are not included directly in the UserData but through the context property is so the user's definitions can never overlap with the uWS.WebSocket object's native methods / properties. While I know it would be a bad practice for someoen to use the same method names as uWS.WebSocket It is much better to make it fundamentally impossible for the user to pollute to the uWS.WebSocket object. For this reason, the user can define any keys in the context and the defined keys will remain isolated in the context object on the uWS.WebSocket object. This is why If you were to ever view the raw version of the WebSocket you will notice that there is only one custom key on it called context which holds the user's custom context data.

I think the problem here is that you assumed the Response.update() method directly passes the context to uWS.WebSocket which is not the case here.

Regardless, I don't know where Alien mode plays a role in any of this.

[joacub]

Yeah that’s the problem your are forcing us to use your context key instead of passing directly to uws, see the upgrade method is already sending that how we want to get this back. We don’t want and we don’t know other libraries will know you are doing that, you should transparent in that case and just send this straight to uws. Basically you are changing the way this work.

The reason why the user's keys are not included directly in the UserData but through the context property is so the user's definitions can never overlap with the uWS.WebSocket object's native methods / properties. While I know it would be a bad practice for someoen to use the same method names as uWS.WebSocket It is much better to make it fundamentally impossible for the user to pollute to the uWS.WebSocket object. For this reason, the user can define any keys in the context and the defined keys will remain isolated in the context object on the uWS.WebSocket object. This is why If you were to ever view the raw version of the WebSocket you will notice that there is only one custom key on it called context which holds the user's custom context data.

I think the problem here is that you assumed the Response.update() method directly passes the context to uWS.WebSocket which is not the case here.

Regardless, I don't know where Alien mode plays a role in any of this.

this is different to the alien mode, i just saw that, that socket.io is not playing right with your upgrade method, i just update to attach the raw uws_instance, so then socket.io can play directly with uws, btw the error still there, seems something that is not compatible with socket.io, and is something should be one fixed by them or maybe is a uws bug, i really dont know where is the fault here yet, ill be investigating if i can and i have the time, but yeah seems something socket.io is maybe not doing right.

[kartikk221]

The problem is that socket.io or whatever implementation you posted above calls req.getHeader where req is the uWS.HttpResponse that is stack allocated. If you are using hyper-express then you must use the Request.headers[name] to retrieve the cached headers and not directly take from uWebsocket.js. That is what's throwing the forbidden error in your case. You cannot use the Request.raw after the initial synchronous callback.

[joacub]

The problem is that socket.io or whatever implementation you posted above calls req.getHeader where req is the uWS.HttpResponse that is stack allocated. If you are using hyper-express then you must use the Request.headers[name] to retrieve the cached headers and not directly take from uWebsocket.js. That is what's throwing the forbidden error in your case. You cannot use the Request.raw after the initial synchronous callback.

yes i was about to comment that, is the use of the getter that is causing this, but without telling anybody is the fault of any one, the example that uWebsocket.js has in their codebase, is with the getter so, in theory should work as per their examples, but yeah with alien mode you can not access no more that i guess.

[joacub]

how i can access that headers without calling you upgrade mode without braking their access forbidden ? can you expose that var ?

or if you update your upgrade_mode to allow the user vars to be what it is we can use that.

[kartikk221]

The Request object from hyper-express has the headers property which contains all headers in lowercase keys, just use that.

[kartikk221]

Yeah it's fine their example because they instantly upgrade but If they were asynchronously upgrading they would have to cache the headers into variables or an object and then use them during the upgrade.

[joacub]

Thanks, i just did that test removing this getters and pass the connection, now is another error in engine.io end. they should do something to be compatible with this mode.

thanks a lot for all your support on this.

[kartikk221]

I would probably not use Alien mode for now since it runs uWS in a separate event loop even synchronous executions aren't safe from the two event loops being out of sync it seems like given the issues you are encountered.

[joacub]

I would probably not use Alien mode for now since it runs uWS in a separate event loop even synchronous executions aren't safe from the two event loops being out of sync it seems like given the issues you are encountered.

yes, i agree with that 100%, it can be out sync at some point, there is not guarantee for that.

[joacub]

you should leave at the use decision, or if you want warm them about it, but leave this decision up to the developer, dont force anybody to use one or either.

[kartikk221]

Yeah, that's the plan, gonna be pushing an update soon where it's disabled by default and a few optimizations.

[joacub]

i manage to have working socket.io with alien mode, just accessing this values in a cached var. everything else is working properly for now.

[joacub]

i just did a pull request:

https://github.com/socketio/engine.io/pull/681

[joacub]

Yeah, that's the plan, gonna be pushing an update soon where it's disabled by default and a few optimizations.

what are that things they are pointing thet hyper-express is not doing right about cork ?

is this related to a performance or something that may be missing in the requests ?

[joacub]

Just saw your issue at https://github.com/uNetworking/uWebSockets.js/issues/907

So the Websocket upgrade fails? If so that is a uWebsockets.js bug because the uWS.HttpResponse object should not be deallocated and uWS.HttpResponse.upgrade() should be callable at any time.

issue was removed

[joacub]

look at this, the owner change the files examples without modifying te timing of the files:

https://github.com/uNetworking/uWebSockets.js/blob/master/examples/UpgradeAsync.js

...... no comments....