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

v3 and v4 close socket if more than 1MB is sent (but v2 is fine) #3946

Closed
Domiii opened this issue May 28, 2021 · 4 comments
Closed

v3 and v4 close socket if more than 1MB is sent (but v2 is fine) #3946

Domiii opened this issue May 28, 2021 · 4 comments
Labels
enhancement New feature or request
Milestone

Comments

@Domiii
Copy link

Domiii commented May 28, 2021

Describe the bug

  • Everytime a certain amunt of data is sent by the client, the socket disconnects instantly.
  • I am using socket.io@4.1.2
  • Client in browser (latest Chrome)
  • Server on Node@16

Steps to reproduce

  • Connect to server
  • Let client send a packet that is at least 1e6 in size (code sample below)
  • You will see it disconnect instantly.

Code Sample

Socket.IO server version: 4.1.2

Server

import { Server } from "socket.io";

const io = new Server(3000, {});

io.on("connection", (socket) => {
  console.log(`connect ${socket.id}`);

  socket.on("disconnect", () => {
    console.log(`disconnected ${socket.id}`);
  });

  socket.on("noop", (data) => {
    console.log(`noop received: ${data.length}`);
  });
});

Socket.IO client version: 4.1.2

Client

import { io } from "socket.io-client";

const socket = io("ws://localhost:3000/", {});

socket.on("connect", () => {
  console.log(`connect ${socket.id}`);

  // NOTE: if you change N to 999000, it will NOT disconnect!
  const N = 1e6;
  const noopData = new Array(N).fill(1).join('');
  socket.emit('noop', noopData);
  console.warn(`noop sent, N=${N}`);
});

socket.on("disconnect", () => {
  console.log("disconnect");
});

Expected behavior

Don't disconnect arbitrarily.

Platform:

  • Device: PC
  • OS: Windows10

Further Analysis

  • I find the issue to also exist in socket.io@3, but not in socket.io@2, which works just fine.
  • I debugged the issue and found that N=1e6 will disconnect, while N=999000 will NOT disconnect.
    • That seems to indicate that whatever bug it is, it is largely tied to some sort of 1MB quota.
  • When putting a breakpoint in the onClose handler in ./node_modules/engine.io-client/lib/transports/websocket.js, you can debug the native WebSocket error event (which is in arguments[0]).
    • It's code is 1005.
    • According to MDN, code 1005 - "Indicates that no status code was provided even though one was expected."
@Domiii Domiii added the bug Something isn't working label May 28, 2021
@Domiii Domiii changed the title v4.1.2 closes socket if more than 1MB is sent (code 1005) v3 an v4 close socket if more than 1MB is sent (code 1005) (but v2 is fine) May 28, 2021
@Domiii Domiii changed the title v3 an v4 close socket if more than 1MB is sent (code 1005) (but v2 is fine) v3 an v4 close socket if more than 1MB is sent (but v2 is fine) May 28, 2021
@Domiii Domiii changed the title v3 an v4 close socket if more than 1MB is sent (but v2 is fine) v3 and v4 close socket if more than 1MB is sent (but v2 is fine) May 28, 2021
@darrachequesne
Copy link
Member

Yes, this is expected, the maxHttpBufferSize value has been decreased from 100 MB to 1MB in v3, you can update it with:

const io = require("socket.io")(httpServer, {
  maxHttpBufferSize: 1e8
});

Reference:

@Domiii
Copy link
Author

Domiii commented May 29, 2021

Thanks for the quick reply!
Would be fantastic if one received a corresponding error message or warning, rather than just a silent shutdown. At least in development mode, it could have some sanity checks, to my mind.
Considering that the close event has an error code, at least that can be reported on.

Can you possibly consider converting this from bug report to feature request?

@darrachequesne
Copy link
Member

Can you possibly consider converting this from bug report to feature request?

Yes, we could indeed make the error more user-friendly 👍

Possibly linked: #3237

@darrachequesne darrachequesne added enhancement New feature or request and removed bug Something isn't working labels Jun 3, 2021
darrachequesne added a commit to socketio/engine.io-client that referenced this issue Apr 11, 2022
The close event will now include additional details to help debugging
if anything has gone wrong.

Example when a payload is over the maxHttpBufferSize value in HTTP
long-polling mode:

```js
socket.on("close", (reason, details) => {
  console.log(reason); // "transport error"

  // in that case, details is an error object
  console.log(details.message); "xhr post error"
  console.log(details.description); // 413 (the HTTP status of the response)

  // details.context refers to the XMLHttpRequest object
  console.log(details.context.status); // 413
  console.log(details.context.responseText); // ""
});
```

Note: the error object was already included before this commit and is
kept for backward compatibility

Example when a payload is over the maxHttpBufferSize value with
WebSockets:

```js
socket.on("close", (reason, details) => {
  console.log(reason); // "transport close"

  // in that case, details is a plain object
  console.log(details.description); // "websocket connection closed"

  // details.context is a CloseEvent object
  console.log(details.context.code); // 1009 (which means "Message Too Big")
  console.log(details.context.reason); // ""
});
```

Example within a cluster without sticky sessions:

```js
socket.on("close", (reason, details) => {
  console.log(details.context.status); // 400
  console.log(details.context.responseText); // '{"code":1,"message":"Session ID unknown"}'
});
```

Note: we could also print some warnings in development for the "usual"
errors:

- CORS error
- HTTP 400 with multiple nodes
- HTTP 413 with maxHttpBufferSize

but that would require an additional step when going to production
(i.e. setting NODE_ENV variable to "production"). This is open to
discussion!

Related:

- socketio/socket.io#3946
- socketio/socket.io#1979
- socketio/socket.io-client#1518
@darrachequesne
Copy link
Member

Closed by socketio/engine.io-client@b9252e2, included in socket.io-client@4.5.0.

socket.on("close", (reason, details) => {
  console.log(reason); // "transport error"

  // in that case, details is an error object
  console.log(details.message); "xhr post error"
  console.log(details.description); // 413 (the HTTP status of the response)

  // details.context refers to the XMLHttpRequest object
  console.log(details.context.status); // 413
  console.log(details.context.responseText); // ""
});

Please reopen if needed!

@darrachequesne darrachequesne added this to the 4.5.0 milestone Oct 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants