Skip to content

Commit 41b1348

Browse files
committed
fix(streams): harden wasm elements per review
Five open review-round findings on the connection elements: - Guard per-line/per-frame debug interpolation behind debug.enabled in wasm-n2k's decode-error handler and j1939-can's frame handler. - Protect j1939-can's start()-failure teardown: connect() also runs from the reconnect timer, where a throw from stop() would kill the process and skip the reconnect. - wasm-n2k-bytes: register txHandler before connect() so the constructor has no initialization-order dependency. - wasm-n2k-bytes: honor noDataReceivedTimeout on the TCP transport with a receive-only idle timer — socket.setTimeout() counts the periodic keepalive writes as activity, so it could never fire while the gateway sends nothing. Armed per connection, re-armed only on received data, cleared in retry and end(). - Maretron help text no longer claims canboatjs handles the framing; the wasm variant shares the block.
1 parent 35b4f54 commit 41b1348

4 files changed

Lines changed: 69 additions & 9 deletions

File tree

packages/server-admin-ui/src/views/ServerConfig/BasicProvider.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1928,7 +1928,7 @@ function NMEA2000({
19281928
</Form.Group>
19291929
<div className="text-muted small mt-1 mb-2">
19301930
Maretron IPG 100 Ethernet gateway. Default TCP port 6543. Uses
1931-
Maretron&apos;s 0xA5-framed binary protocol (handled by canboatjs).
1931+
Maretron&apos;s 0xA5-framed binary protocol.
19321932
</div>
19331933
</div>
19341934
)}

packages/streams/src/j1939-can.ts

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,10 @@ export default class J1939Can extends Transform {
107107
data: msg.data
108108
})
109109
} catch (err: unknown) {
110-
const message = err instanceof Error ? err.message : String(err)
111-
this.debug(`[frame] ${message}`)
110+
if (this.debug.enabled) {
111+
const message = err instanceof Error ? err.message : String(err)
112+
this.debug(`[frame] ${message}`)
113+
}
112114
}
113115
})
114116
channel.addListener('onStopped', () => {
@@ -130,8 +132,19 @@ export default class J1939Can extends Transform {
130132
channel.start()
131133
} catch (err: unknown) {
132134
const message = err instanceof Error ? err.message : String(err)
133-
channel.removeAllListeners()
134-
channel.stop()
135+
// connect() also runs from the reconnect timer, where an escaping
136+
// throw from the half-started channel's teardown would take the
137+
// process down and skip the reconnect.
138+
try {
139+
channel.removeAllListeners()
140+
channel.stop()
141+
} catch (stopErr: unknown) {
142+
if (this.debug.enabled) {
143+
this.debug(
144+
`[stop] ${stopErr instanceof Error ? stopErr.message : String(stopErr)}`
145+
)
146+
}
147+
}
135148
this.channel = null
136149
if (this.options.providerId) {
137150
this.options.app.setProviderError?.(this.options.providerId, message)

packages/streams/src/wasm-n2k-bytes.ts

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ interface WasmN2kBytesOptions {
3131
/** Serial transport instead of TCP (e.g. an NGT-1 dongle). */
3232
device?: string
3333
baudrate?: number
34+
noDataReceivedTimeout?: string | number
3435
byteKind: 'ngt1' | 'maretron-ipg'
3536
password?: string
3637
providerId?: string
@@ -71,6 +72,7 @@ function requireWasm(): WasmByteApi {
7172

7273
const RECONNECT_DELAY = 3000
7374
const KEEPALIVE_INTERVAL = 20000
75+
const DEFAULT_IDLE_TIMEOUT_SECONDS = 60
7476

7577
interface ByteTransport {
7678
write(data: Buffer): void
@@ -84,6 +86,8 @@ export default class WasmN2kBytes extends Transform {
8486
private socket: ByteTransport | null = null
8587
private keepaliveTimer: NodeJS.Timeout | null = null
8688
private reconnectTimer: NodeJS.Timeout | null = null
89+
private idleTimer: NodeJS.Timeout | null = null
90+
private idleMs = 0
8791
private stopped = false
8892
private readonly txHandler: (pgn: unknown) => void
8993
private readonly debug: DebugLogger
@@ -95,8 +99,6 @@ export default class WasmN2kBytes extends Transform {
9599
const createDebug = options.createDebug ?? require('debug')
96100
this.debug = createDebug('signalk:streams:wasm-n2k-bytes')
97101

98-
this.connect()
99-
100102
this.txHandler = (pgn: unknown) => {
101103
try {
102104
const bytes = this.decoder.encodeFrame(JSON.stringify(pgn), true)
@@ -109,6 +111,8 @@ export default class WasmN2kBytes extends Transform {
109111
}
110112
options.app.on('nmea2000JsonOut', this.txHandler)
111113
options.app.emit('nmea2000OutAvailable')
114+
115+
this.connect()
112116
}
113117

114118
private status(msg: string): void {
@@ -118,6 +122,31 @@ export default class WasmN2kBytes extends Transform {
118122
this.debug(msg)
119123
}
120124

125+
private clearIdleTimer(): void {
126+
if (this.idleTimer) {
127+
clearTimeout(this.idleTimer)
128+
this.idleTimer = null
129+
}
130+
}
131+
132+
private armIdleTimer(): void {
133+
this.clearIdleTimer()
134+
if (this.idleMs <= 0) {
135+
return
136+
}
137+
const socket = this.socket
138+
this.idleTimer = setTimeout(() => {
139+
this.idleTimer = null
140+
if (!socket || this.socket !== socket) {
141+
return
142+
}
143+
this.debug.enabled && this.debug('Idle timeout, closing socket')
144+
// destroy(), not end(): the peer is presumed gone, and only
145+
// destroy fires 'close' immediately for the retry path.
146+
socket.destroy()
147+
}, this.idleMs)
148+
}
149+
121150
private connect(): void {
122151
if (this.stopped) {
123152
return
@@ -168,12 +197,26 @@ export default class WasmN2kBytes extends Transform {
168197
}
169198
const tcp = new Socket()
170199
this.status(`Connecting to ${host}:${port}`)
200+
// Receive-only idle timer: a half-open connection raises neither
201+
// 'error' nor 'close', so without it the retry path never runs.
202+
// socket.setTimeout() cannot serve here — it counts the periodic
203+
// keepalive writes as activity, so it would never fire while the
204+
// gateway sends nothing. Re-armed only in the data handler,
205+
// cleared in retry and end().
206+
const parsedTimeout = Number.parseInt(
207+
(this.options.noDataReceivedTimeout + '').trim()
208+
)
209+
this.idleMs =
210+
(isNaN(parsedTimeout) ? DEFAULT_IDLE_TIMEOUT_SECONDS : parsedTimeout) *
211+
1000
171212
tcp.connect(port, host, () => onOpen(tcp))
172213
socket = tcp
173214
}
174215
this.socket = socket
216+
this.armIdleTimer()
175217

176218
socket.on('data', (buf: Buffer) => {
219+
this.armIdleTimer()
177220
// Everything here runs inside a socket event handler, where an
178221
// uncaught throw takes the process down. Malformed framing from
179222
// a gateway must degrade to a logged error, not a crash.
@@ -206,6 +249,7 @@ export default class WasmN2kBytes extends Transform {
206249
})
207250

208251
const retry = (why: string) => {
252+
this.clearIdleTimer()
209253
if (this.keepaliveTimer) {
210254
clearInterval(this.keepaliveTimer)
211255
this.keepaliveTimer = null
@@ -252,6 +296,7 @@ export default class WasmN2kBytes extends Transform {
252296
clearInterval(this.keepaliveTimer)
253297
this.keepaliveTimer = null
254298
}
299+
this.clearIdleTimer()
255300
this.socket?.destroy()
256301
this.socket = null
257302
super.end()

packages/streams/src/wasm-n2k.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,10 @@ export default class WasmN2k extends Transform {
107107
options.j1939 === true ? { j1939: true } : undefined
108108
)
109109
this.fromPgn.on('error', (line: unknown, err: unknown) => {
110-
const message = err instanceof Error ? err.message : String(err)
111-
this.debug(`[error] ${String(line)} ${message}`)
110+
if (this.debug.enabled) {
111+
const message = err instanceof Error ? err.message : String(err)
112+
this.debug(`[error] ${String(line)} ${message}`)
113+
}
112114
options.app.emit('canboatjs:error', err)
113115
})
114116

0 commit comments

Comments
 (0)