Skip to content

fix(node): harden websocket transport reliability - #92

Merged
M09Ic merged 7 commits into
masterfrom
fix/node-websocket-reliability
Aug 10, 2026
Merged

fix(node): harden websocket transport reliability#92
M09Ic merged 7 commits into
masterfrom
fix/node-websocket-reliability

Conversation

@h3zh1

@h3zh1 h3zh1 commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Summary

  • reset exponential reconnect backoff only after an accepted WebSocket session remains stable
  • detect silent peers with bounded ping, pong, read, and write deadlines
  • report actionable TLS, DNS, TCP, HTTP handshake, timeout, and WebSocket close diagnostics
  • advertise a process-scoped node identity that stays stable across reconnects and changes after restart
  • preserve correlated enrollment rejection codes and messages returned by the server

User-visible behavior

  • repeated short-lived connections continue backing off instead of resetting after every handshake
  • half-open WebSocket connections are detected and reconnected without waiting indefinitely
  • certificate, token, endpoint, network, and peer-close failures produce specific Runner logs
  • AOP hubs can distinguish a reconnecting process from a different process reusing the same node ID
  • enrollment conflicts such as ALREADY_EXISTS reach the Runner log instead of becoming a generic acceptance error

Commit structure

  1. fix(node): reset reconnect backoff after stable sessions
  2. fix(node): enforce websocket liveness deadlines
  3. fix(node): classify websocket connection failures
  4. feat(node): advertise runner process instance identity
  5. fix(node): surface enrollment rejection reasons

Verification

  • go test ./pkg/node ./core/deps at each commit
  • go test -race -count=1 ./pkg/node
  • go vet ./...
  • go mod tidy -diff
  • git diff --check origin/master...HEAD

Test note

The unfiltered go test ./... run on macOS still fails the pre-existing TestShellCommandRedirectionAndInvocationContext assertion because the shell resolves /var/... as /private/var/.... The same failure reproduces on unmodified origin/master; all pkg/node tests pass.

Integration

Cairn must update its AIScan module version after this branch is published or merged before it consumes the final commit hashes.

Deferred

  • the previously recorded WebSocket 512 KiB message limit is not addressed here
  • Cairn Server connection-pool and enrollment-policy changes remain in the Cairn repository

@h3zh1
h3zh1 marked this pull request as ready for review August 10, 2026 04:51
@M09Ic

M09Ic commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Pushed bd6e7c6d to simplify the runner WebSocket reliability changes after auditing every new package variable, connection field, config field, and function parameter.

Key changes:

  • removed the mutable reconnect timing hooks and the test-only Dialer / Liveness fields from production config
  • reduced webSocketEnvelopeStream lifecycle state to the connection, write mutex, and codec mode; heartbeat lifetime is now owned by the connection context
  • close the stream after an application write failure so a blocked Recv wakes immediately and reconnects, instead of waiting for the read deadline
  • calculate ping deadlines from time.Now() at the actual write rather than from a possibly stale ticker timestamp
  • generate instance_id inside each RunToolNode invocation, keeping it stable across that runner's reconnects without sharing mutable process-global identity
  • keep fixed liveness/backoff values as constants and test behavior without production timing injection
  • preserve raw DNS/TCP/timeout errors while retaining actionable TLS, HTTP handshake, WebSocket close, and enrollment rejection diagnostics

The cleanup removes 220 net lines from the previous PR head. The PR is now 7 changed files with +599/-19, down from 9 files with +818/-18.

Verified locally:

  • go test -race -count=1 ./pkg/node
  • go test -count=1 ./core/deps
  • go vet ./...
  • go mod tidy -diff
  • git diff --check

@M09Ic

M09Ic commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

CI follow-up: the new workflow run is blocked during actions/checkout, before any AIScan lint/test command runs. The current master merge result references web/frontend/cyber-ui commit 2a00bac673570fef65535c4753ffc4f6a95ff502, which the submodule remote reports as "not our ref".

This reproduces on the current base commit 0b4850a6: its own CI run failed at the same checkout step: https://github.com/chainreactors/aiscan/actions/runs/31357987246

PR run: https://github.com/chainreactors/aiscan/actions/runs/31359589270

The runner changes themselves continue to pass the local race, tests, vet, tidy, and diff checks listed above. I have not changed the unrelated submodule pointer in this PR.

@M09Ic

M09Ic commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Follow-up cleanup pushed in e2d5c26d, focused specifically on recovery from a blocked transport rather than adding a tunable liveness framework.

  • only two independent policy values remain: a 30-second keepalive interval and a 10-second write-block timeout
  • websocketPongWait is derived as three heartbeat intervals (90 seconds)
  • reconnectStableAfter is derived as one complete liveness window plus one heartbeat interval (120 seconds), so a fully silent peer times out before it can reset reconnect backoff
  • removed the stream write mutex: application envelopes already pass through one sendCh writer, matching the EnvelopeStream single-Send contract; Gorilla explicitly permits WriteControl concurrently
  • the recovery path remains singular: a blocked write reaches its deadline, the write error closes the stream, blocked Recv wakes, and the outer loop reconnects with backoff

Re-verified with go test -race -count=1 ./pkg/node, go test -count=1 ./core/deps, go vet ./..., go mod tidy -diff, and git diff --check.

@M09Ic
M09Ic merged commit fa9acb9 into master Aug 10, 2026
11 of 14 checks passed
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 this pull request may close these issues.

2 participants