FIX unhandled promises on transport.send()

[aricart]

These could bubble up and be trapped by the runtime, resulting in the client crashing.

FIXES #271

[kozlovic]

LGTM

[intech]

Errors suppression was never considered a good pattern - wasn't it?

[intech]

@aricart @kozlovic ping

[aricart]

In this particular case it slipped through - the correct handling of the error was elsewhere but those promises were forgotten :(

[aricart]

In case it is not clear, these promises fail when the socket already failed, which is handled by the transport implementation. The rejections of the write are typically subsequent to this and can be safely ignored, as the client will enter its reconnect logic. As usual, this is core NATS so it is at most once delivery.

For deno the detection happens on a read error
https://github.com/nats-io/nats.deno/blob/main/src/deno_transport.ts#L187

For node/websocket the event handlers on the socket deal with it
https://github.com/nats-io/nats.js/blob/main/src/node_transport.ts#L278
https://github.com/nats-io/nats.ws/blob/main/src/ws_transport.ts#L144

[intech]

@aricart A more attractive solution would be to create an error event and emit such errors so that they can always be logged and processed if necessary.

[aricart]

The disconnect is reported via the status mechanism. If your app requires to know if your messages arrived, you use request reply.

[arjenvdhave]

Hi @aricart

I have this error:

Error: write EPIPE
at afterWriteDispatched (node:internal/stream_base_commons:160:15)
at writeGeneric (node:internal/stream_base_commons:151:3)
at Socket._writeGeneric (node:net:817:11)
at Socket._write (node:net:829:8)
at writeOrBuffer (node:internal/streams/writable:389:12)
at _write (node:internal/streams/writable:330:10)
at Socket.Writable.write (node:internal/streams/writable:334:10)
at NodeTransport.send (/usr/src/app/node_modules/nats/lib/src/node_transport.js:327:21)
at ProtocolHandler.processInfo (/usr/src/app/node_modules/nats/lib/nats-base-client/protocol.js:425:32)
at ProtocolHandler.push (/usr/src/app/node_modules/nats/lib/nats-base-client/protocol.js:461:22) {
errno: -32,
code: 'EPIPE',
syscall: 'write'

Could it be that the .catch should also be added to this call:
https://github.com/nats-io/nats.deno/blob/main/nats-base-client/protocol.ts#L509

[aricart]

@arjenvdhave that one is in catch but not awaited, may need to modify the transport interface. - really want to know when that fails right there.

[arjenvdhave]

Yep me to :)
For some reason I only get this in my app when it is running in a docker container on our k8s cluster.
I still haven't got a clue what the cause is.

Other services using the same setup don't have this issue.

[aricart]

@arjenvdhave the network overlay is possibly the problem, right now trying to remove the async nature of send - as I am interested in a possible exception that happens as part of the write operation, but not the actual write succeeding...
Do you have a repeatable case, if I publish a bundle for you to try?

[arjenvdhave]

I hacked the catch call in my docker image.
I now get this error:

error: NatsError: DISCONNECT
- error.js:100 Function.errorForCode
[app]/[nats]/lib/nats-base-client/error.js:100:16
- protocol.js:175
[app]/[nats]/lib/nats-base-client/protocol.js:175:40
- Array.forEach
- protocol.js:174 ProtocolHandler.resetOutbound
[app]/[nats]/lib/nats-base-client/protocol.js:174:15
- protocol.js:195 ProtocolHandler.prepare
[app]/[nats]/lib/nats-base-client/protocol.js:195:14
- protocol.js:244 ProtocolHandler.<anonymous>
[app]/[nats]/lib/nats-base-client/protocol.js:244:31
- Generator.next
- protocol.js:34
[app]/[nats]/lib/nats-base-client/protocol.js:34:75
- new Promise
- protocol.js:13 __awaiter
[app]/[nats]/lib/nats-base-client/protocol.js:13:16

And after that it retries and connects without issues

[arjenvdhave]

@aricart i can reproduce the issues on my cluster.
If you have "fix" i can try it.

I however do use the nats-js lib, not this deno directly

[aricart]

@arjenvdhave give me a few, posting the branch for the deno side, now need to change the usage on nats.js