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

Errors on socket end, Cannot read properties of undefined (reading 'end') #560

Closed
Cherry opened this issue Feb 22, 2023 · 19 comments
Closed

Comments

@Cherry
Copy link

Cherry commented Feb 22, 2023

We recently upgraded from version 1 to the latest version 2 of the NATS.js client, and over the last few weeks have been seeing a growing number of the following issue being reported in our Sentry:

Cannot read properties of undefined (reading 'end')

With the following stack:

Raw:

TypeError: Cannot read properties of undefined (reading 'end')
  File "<snip>/node_modules/nats/lib/src/node_transport.js", line 274, col 29, in <anonymous>
    this.socket.end();
  File "node:internal/streams/writable", line 500, col 5, in afterWrite
  File "node:internal/streams/writable", line 480, col 7, in onwrite
  File "node:internal/stream_base_commons", line 87, col 12, in WriteWrap.onWriteComplete [as oncomplete]
  File "node:_tls_wrap", line 581, col 11, in Socket.done
  File "node:events", line 628, col 26, in Object.onceWrapper
  File "node:events", line 513, col 28, in Socket.emit
  File "node:domain", line 489, col 12, in Socket.emit
  File "node:net", line 322, col 12, in TCP.<anonymous>

As this happens, wee see the reconnecting event from connection.status() fire, and it's also sometimes accompanied with a NatsError: 'Stale Connection' error.

The issue itself here doesn't seem to impact anything most of the time, and the client reconnects without issue. But we have sometimes seen cases where the client hangs and requires us to completely restart it.

@aricart
Copy link
Member

aricart commented Feb 22, 2023

What version of the client are you on?

@aricart
Copy link
Member

aricart commented Feb 22, 2023

This looks to me as a Proxy/Kubernetes or something that is holding on to the socket and not properly closing it. Stale connection means that the pings expected from the server were not received, but the connection is still open.

@Cherry
Copy link
Author

Cherry commented Feb 22, 2023

We were using nats client version 2.11.0, but I just bumped to nats 2.12.1.

As for the Proxy/Kubernetes, we are using Docker's port forwarding for hosting our NATS cluster, via Docker Swarm.

@aricart
Copy link
Member

aricart commented Feb 22, 2023

could try and put a guard on that socket reference in case that it was nullfied. I am thinking that perhaps some of the events are not coming in the right order (or the expected order). Just to be clear, do you think the client is hanging due to a crash, or is it that the socket is partially open for some unknown reason.

@aricart
Copy link
Member

aricart commented Feb 22, 2023

If I make you a patch, could you partially deploy it and could monitor if it makes it better?

@Cherry
Copy link
Author

Cherry commented Feb 22, 2023

Yeah that's what it feels like to me too.

I'm honestly not sure if the client hang is related to this - we're still trying to get to the bottom of that. It's happening very irregularly and unpredictably, sometimes accompanied with this error and sometimes without.

I'd be happy to deploy a patch and monitor and get back to you. This may take a few days to gather reports and monitor Sentry for changes.

@aricart
Copy link
Member

aricart commented Feb 22, 2023

sounds like a plan. Give me a bit.

aricart added a commit that referenced this issue Feb 22, 2023
@aricart
Copy link
Member

aricart commented Feb 22, 2023

@Cherry npm install nats@2.12.2-1

@Cherry
Copy link
Author

Cherry commented Feb 22, 2023

Thank you very much! I've deployed this and will report back in a few days as we see any changes. 🤞

@Cherry
Copy link
Author

Cherry commented Mar 6, 2023

As an update here, we haven't seen this specific error occur again with nats@2.12.2-1, which is great! We're still experiencing some weird Stale Connection errors here and there, but I'll need to dig into this more before following up.

I think the changes in 2.12.2-1 are good though!

@aricart
Copy link
Member

aricart commented Mar 7, 2023

Awesome!

@aricart
Copy link
Member

aricart commented Mar 9, 2023

@Cherry I am going to close this issue, but feel free to reopen or create a new issue for anything else.

@aricart aricart closed this as completed Mar 9, 2023
@Cherry
Copy link
Author

Cherry commented Mar 10, 2023

Thanks @aricart. Will the next version include the change in https://github.com/nats-io/nats.js/tree/fix-560, so we can get back on a mainstream release?

@aricart
Copy link
Member

aricart commented Mar 10, 2023

Believe it is there

@aricart
Copy link
Member

aricart commented Mar 10, 2023

If not I'll release

@Cherry
Copy link
Author

Cherry commented Mar 10, 2023

From what I can tell, the fix doesn't appear to be in main just yet

@aricart
Copy link
Member

aricart commented Mar 11, 2023

@Cherry you are right - taking care of it.

aricart added a commit that referenced this issue Mar 11, 2023
* [FIX] added guard to see if it fixes  #560

* bump version
@aricart
Copy link
Member

aricart commented Mar 11, 2023

@Cherry npm install nats@latest!

@Cherry
Copy link
Author

Cherry commented Mar 11, 2023

Thank you!

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

2 participants