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

reader closed during delivery tests #194

Closed
h0jeZvgoxFepBQ2C opened this issue Apr 20, 2023 · 10 comments · Fixed by nats-io/nats-server#4084
Closed

reader closed during delivery tests #194

h0jeZvgoxFepBQ2C opened this issue Apr 20, 2023 · 10 comments · Fixed by nats-io/nats-server#4084

Comments

@h0jeZvgoxFepBQ2C
Copy link

h0jeZvgoxFepBQ2C commented Apr 20, 2023

Right now we are testing nats.ws and it occured to us, that when getting flooded with bigger messages (300 messages x 400 kilobytes), following error happens:

Bildschirm­foto 2023-04-20 um 17 26 22

This doesn't happen when we send the same content, just less characters.
After this error, the client is not receiving anymore.

Also the connection monitoring doesn't work, there is no disconnection event triggered, so the client also doesn't reconnect?

This is our code:

  const connectionOptions = {
    servers: ["ws://localhost:4333"],
    debug: true,
    maxReconnectAttempts: -1,
    reconnectTimeWait: 2000,
    timeout: 3000
  }

useEffect(() => {
    setConnectionState("connecting")

    const connectAndSubscribe = async () => {
      try {
        const nc = await connect(connectionOptions)
        setConnectionState("connected")
        console.info(`connected ${nc.getServer()}`);

        channels.map(channelName => {
          const sub = nc.subscribe(channelName);
          console.log("Subscribing to ", channelName);
          (async () => {
            for await (const m of sub) {
              const sc = StringCodec();
              try {
                const result = JSON.parse(sc.decode(m.data))
                onMessageCompleted("nats", result.id, result)
              } catch(ex) {
                console.error(ex)
              }
            }
            console.log("subscription closed");
          })();
        });

        (async () => {
          for await (const s of nc.status()) {
            console.info("mmmmm", s, s.type);
            switch(s.type) {
            case "disconnect":
              setConnectionState("disconnected");
              break;
            case "reconnect":
              setConnectionState("connected");
              break;
            case "reconnecting":
              console.log("reconnectingreconnectingreconnectingreconnectingreconnecting", connectionRetries);
              setConnectionRetries(connectionRetries + 1);
              setConnectionState("connecting");
              break;
            case "error":
              setConnectionState("failed");
              break;
            }
          }
        })();

        nc.closed().then((err) => {
          if (err) {
            console.log('Connection error:', err.message);
            handleConnectionError();
          } else {
            console.log('Connection closed');
          }
        });

        window.nats = nc

      } catch (err) {
        console.log('Failed to connect:', err.message);
        handleConnectionError()
      }
    }

    const handleConnectionError = () => {
      setTimeout(() => {
        connectAndSubscribe();
      }, connectionOptions.reconnectTimeWait);
    }
    connectAndSubscribe()

    return () => {
      if (window.nats) {
        window.nats.close();
      }
    };
  }, [])
@aricart
Copy link
Member

aricart commented Apr 20, 2023

@h0jeZvgoxFepBQ2C what server version are you using?

@h0jeZvgoxFepBQ2C
Copy link
Author

h0jeZvgoxFepBQ2C commented Apr 20, 2023

Version: 2.9.16 run via Docker desktop.

Here is our config file:

# Client port of 4222 on all interfaces
port: 4222

# HTTP monitoring port
monitor_port: 8222
debug:   true
max_payload: 64MB

# This is for clustering multiple servers together.
cluster {
  # It is recommended to set a cluster name
  name: "my_cluster"

  # Route connections to be received on any interface on port 6222
  port: 6222

  # Routes are protected, so need to use them with --routes flag
  # e.g. --routes=nats-route://ruser:T0pS3cr3t@otherdockerhost:6222
  authorization {
    user: ruser
    password: T0pS3cr3t
    timeout: 2
  }

  # Routes are actively solicited and connected to from this server.
  # This Docker image has none by default, but you can pass a
  # flag to the nats-server docker image to create one to an existing server.
  routes = []
}

websocket {
  port: 4333
  no_tls: true,
}

and here our docker-compose entry:

  nats:
    image: nats
    ports:
      - "4222:4222"
      - "4333:4333"
      - "8222:8222"
    volumes:
        - ./cluster/dev/nats/nats.conf:/nats-server.conf

@aricart
Copy link
Member

aricart commented Apr 20, 2023

go back to 2.9.15 - I think there's an issue on the server, need to investigate.

@aricart
Copy link
Member

aricart commented Apr 20, 2023

Can you give me more info on what your code is doing, need a test to reproduce.

@h0jeZvgoxFepBQ2C
Copy link
Author

Yep thanks, this was the issue. 2.9.15 works fine. Will give you some more code in some minutes.

@h0jeZvgoxFepBQ2C
Copy link
Author

We are using the ruby gem for sending and a react app for receiving (code above):

    size = 4.kilobytes # or 400.kilobytes, which didn't work in 2.9.16
    300.times do |i|
      msg = { i: i, type: event, data: message}.to_json

      nats = NATS.connect({
        servers: ["nats://127.0.0.1:4222"],
        connect_timeout: 30, 
        write_timeout: 30, 
        read_timeout: 30
      })
      nats.publish("test", msg)
      nats.drain
    end

@h0jeZvgoxFepBQ2C
Copy link
Author

But with 2.9.15 everything works fine, thank you so much, also for the ultra fast response ❤️ ❤️ ❤️

@h0jeZvgoxFepBQ2C
Copy link
Author

Shall I leave this issue open until this gets fixed in the next version or shall I close it?

@aricart
Copy link
Member

aricart commented Apr 20, 2023

@h0jeZvgoxFepBQ2C server group is looking at it

derekcollison added a commit to nats-io/nats-server that referenced this issue Apr 21, 2023
This extends the previous work in #3733 with the following:

1. Remove buffer coalescing, as this could result in a race condition
during the `writev` syscall in rare circumstances
2. Add a third buffer size, to ensure that we aren't allocating more
than we need to without coalescing
3. Refactor buffer handling in the WebSocket code to reduce allocations
and ensure owned buffers aren't incorrectly being pooled resulting in
further race conditions

Fixes nats-io/nats.ws#194.

Signed-off-by: Neil Twigg <neil@nats.io>
@h0jeZvgoxFepBQ2C
Copy link
Author

Thank you so much ❤️

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

Successfully merging a pull request may close this issue.

2 participants