engine.io icon indicating copy to clipboard operation
engine.io copied to clipboard

Downgrade engine_socket transport error "polling request overlap" to 4xx from 5xx

Open pubudu08 opened this issue 3 years ago • 1 comments

Describe the bug

Hi Peeps,

I am facing an issue where the sockets are in polling mode, the engine io long-polling algorithm will error out when you have two open GET requests. As of now, the error is treated as 500 error.

This could happen when a client has a faulty configuration in place. So, I am proposing we should treat this error as 400/406/409 code where it is used to tell the client that a fault has taken place on THEIR side.

https://github.com/socketio/engine.io/blob/4c306af9458d9caab36c7f8294d8c252b273b8dc/lib/transports/polling.ts#L115-L122

To Reproduce

The issue is similar to https://github.com/socketio/engine.io/issues/166 but restricted to polling only. No upgrade to WebSocket is allowed in my case.

You can reproduce the issue in the following ways

  • Using a reverse proxy to proxy WebSocket reproduced
  • Use a browser(firefox) to block WebSocket transport reproduced
  • Using a socket client to proxy WebSocket: not-reproduced
    • transports: [ "polling"]

Once the browser shows the xhr requests(GET) as pending state, copy the request as curl. Execute the curl request via the terminal to reproduce the issue.

Engine.IO client version: 6.1.0

Expected behavior

it indicates a faulty implementation of the engine io long-polling algorithm when you have two open GET requests. The proposed Idea is that we could downgrade data request overlap from client error code listed below as 4xx error instead of 500

https://github.com/socketio/engine.io/blob/4c306af9458d9caab36c7f8294d8c252b273b8dc/lib/transports/polling.ts#L119

potential error codes - 400/406/409

Thanks!

pubudu08 avatar Jul 14 '22 03:07 pubudu08

One way i'd propose to solve this is to make the behaviour of this configurable/overridable so that we don't introduce a breaking change.

carera avatar Jul 14 '22 04:07 carera

That sounds reasonable, let's schedule this for the next major version. :+1:

darrachequesne avatar Nov 20 '22 02:11 darrachequesne

Done in https://github.com/socketio/engine.io/commit/911d0e35757ea9ee93d1807c401c734661615e96, included in version 6.4.2.

darrachequesne avatar May 02 '23 00:05 darrachequesne