Repository navigation
fix(host-service): re-register the host when the relay refuses it - #8907
RohithReddy20 wants to merge 4 commits into
Conversation
The relay checks the host's access row on every control connect. When that row disappears while the host-service is running (host deleted, member removed and re-invited), every reconnect is refused with 4403. `host.ensure` only ran once per process, and the tunnel client treated 4403 like any transport drop, so partysocket kept retrying every ~5s for days with no way to recover short of a manual restart. Meanwhile `superset status` still reported the host as registered. The tunnel client now stops partysocket's retry cycle on 4403 and hands the close reason to a new `onForbidden` hook, with a single in-flight guard so repeated closes do not stack. The watchdog defers while that recovery runs. `connectRelay` wires the hook to the same registration loop the boot path uses: it records the failure so health.check tells the truth, calls `host.ensure` again with the existing backoff, and the client reconnects once registration lands. The machine id and routing key do not change, so the existing tunnel is reused. A regression test runs a local relay that closes the first control connect with 4403 and asserts the hook runs once before the second connect arrives. Fixes ParadevAI#8445
The relay maps an access check it cannot complete (API unreachable from the relay) to the same 4403 as a missing registration. With immediate re-registration, a host that can still reach the API would re-run host.ensure and reconnect in a zero-delay loop. Consecutive 4403 closes without an intervening open now wait 30s, 60s, ... up to 5 min before the next connect; the first refusal after an open still recovers at once, so a lost registration row is repaired immediately.
The relay completes the WebSocket upgrade before it closes a refused control connect with 4403, so `open` fired on every refusal and reset the refusal counter. Repeated refusals then reconnected, and re-ran host.ensure, with no delay. Reset the counter on the first inbound message instead: a refused socket never receives one. The re-registration hook pauses reconnects and the stuck-control watchdog until it settles, but host.ensure had no deadline. The API client sets no timeout, and the only signal fired on shutdown, so one stalled request kept the tunnel down. Give each attempt a 30 s deadline so a stalled request is cancelled and retried on the existing backoff. Tests: the backoff test now opens each refused socket before closing it, and only a received message resets the delay. A new connect test checks that a stalled host.ensure is cancelled at its deadline.
|
All contributors have signed the CLA ✍️ ✅ |
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Registration and relay setup packages/host-service/src/tunnel/connect.ts, packages/host-service/src/tunnel/connect.test.ts |
registerHost retries registration with a 30-second per-attempt timeout and supports caller cancellation. connectRelay resolves the relay and creates the tunnel after registration succeeds. Its forbidden-close callback attempts registration again. The test covers cancellation during a pending registration attempt. |
Forbidden-close recovery packages/host-service/src/tunnel/tunnel-client.ts, packages/host-service/src/tunnel/tunnel-client.test.ts, packages/host-service/src/tunnel/tunnel-client.integration.test.ts |
When configured with onForbidden, the client handles code 4403 closes through the callback before reconnecting. Repeated forbidden closes use delays that start immediately and then grow from 30 seconds to a five-minute cap. Incoming control messages reset the retry count. Unit and real-socket tests cover reconnection and callback behavior. |
Priority:
Estimated code review effort: 3 (Moderate) | ~25 minutes
Change: Bug fix · Severity of issue fixed: Medium
Sequence Diagram(s)
sequenceDiagram
participant Relay
participant TunnelClient
participant registerHost
participant HostAPI
Relay->>TunnelClient: Close control connection with code 4403
TunnelClient->>registerHost: Invoke onForbidden with close reason
registerHost->>HostAPI: Call host.ensure.mutate
HostAPI-->>registerHost: Return registration result
registerHost-->>TunnelClient: Complete recovery callback
TunnelClient->>Relay: Reconnect control connection
Possibly related PRs
- ParadevAI/Paradev#8900: Changes the same tunnel recovery flow with forbidden-close handling, re-registration, and refusal backoff.
Merge Risk: 
Minimal · up to c3773
-
The reviewed recovery path addresses relay refusals without an established merge-blocking issue.
Pre-merge checks: 
4 · 
1
-
Failed checks: 1 warning Passed checks: 4 passed-
Full details: Title check
-
Explanation
The title "fix(host-service): re-register the host when the relay refuses it" follows conventional commit format and names the primary change. The changeset adds a registerHost function and an onForbidden recovery path that re-runs host.ensure after a 4403 forbidden close from the relay. The title is specific and matches the objectives of the PR.
Full details: Description check
-
Explanation
The description follows the template's main sections. It covers What & why with a linked issue (fixes
#8445), a How I tested it section with concrete steps and results, and a checklist. The checklist items are marked as complete. The description does not include screenshots, which the template prefers for user-visible changes. This host-service change is not a UI change, so the gap is minor. The description also does not state whether the 4403 check uses the not_registered reason from the linked issue, and it does not say how failed re-registration is reported through health.check.
Full details: Linked Issues check
-
Explanation
The changes satisfy the coding requirements in [
#8445].TunnelClientintercepts relay close code 4403, invokes recovery, and delays reconnects with the required 0, 30, 60-second sequence up to five minutes. Recovery callsregisterHost, which rerunshost.ensure, uses a 30-second attempt deadline, retries with 30-second-to-five-minute backoff, and records failures for health reporting. The client resets the refusal counter after an inbound control message. Tests cover the recovery ordering, backoff, reset, and registration deadline.
Full details: Out of Scope Changes check
-
Explanation
The reviewed changes stay within [
#8445]. The production changes implement host re-registration after relay refusal, retry backoff, timeout handling, and the related tunnel behavior. The added tests verify these behaviors. No unrelated production change is established by the supplied evidence.
-
-
Finishing Touches: 💡 1
-
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
[Medium impact] The changes since the previous review appear safe to merge. SummaryThe PR re-registers hosts after the relay refuses a connection.
Diagram%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Relay closes with 4403] --> B[Pause reconnects]
B --> C[Record registration failure]
C --> D[Retry host.ensure until success or shutdown]
D --> E[Wait before reconnecting after repeated refusals]
E --> F[Reconnect to relay]
F --> G[First control message resets refusal count]
Reviews (2) · Last reviewed commit: "fix(host-service): trim the relay refusa..." · Reviewed by Greptile |
|
@coderabbitai review |
|
@greptileai review |
|
|
I have read the CLA Document and I hereby sign the CLA |
* chore: seed CLA signature file * @tanxarx has signed the CLA in #8881 * @ozeitis has signed the CLA in #8888 * @shubhankiit has signed the CLA in #8894 * @chrisfentiman has signed the CLA in #8910 * @RohithReddy20 has signed the CLA in #8907 * @larcane97 has signed the CLA in #8922 * @moizghumann has signed the CLA in #8935 * @andyst-dev has signed the CLA in #7930 * @svenpeeters has signed the CLA in #8943 * @kenil-kanani has signed the CLA in #8940 * @akshatcoder-hash has signed the CLA in #8963 * @NilotpalK has signed the CLA in #8995 * @serhii-kucherenko has signed the CLA in #9000 * @tonydzi has signed the CLA in #9002 * @adaam2 has signed the CLA in #9038 * @tsavo-at-pieces has signed the CLA in #9059 * @ganeshmshetty has signed the CLA in #9130 * @aaronrosenthal has signed the CLA in #9135 * @alexnault has signed the CLA in #9138 * @kunalgoesbyken has signed the CLA in #9235 * @Joaco-gavernet has signed the CLA in #9236 * @theblondealex has signed the CLA in #8240 * @talarczykco has signed the CLA in #7274 * @throw99 has signed the CLA in #7906 * @lacherogwu has signed the CLA in #9310 * @KGKallasmaa has signed the CLA in #9316 * @Niltonsf has signed the CLA in #9331 * @Anmol1578 has signed the CLA in #9351 * @RyoKusnadi has signed the CLA in #9317 * @thedotmack has signed the CLA in #9414 * @abudker has signed the CLA in #9439 * @sanchitmonga22 has signed the CLA in #9443 * @Bruno-rose has signed the CLA in #9446 * @Ashad001 has signed the CLA in #9487 * @vivek-pothina has signed the CLA in #8359 * @colingreig has signed the CLA in #9489 * @anmol0b has signed the CLA in #9492 * @resteqs has signed the CLA in #9506 * @pgilmore has signed the CLA in #7878 * fix(desktop): name the DMG from the display name The DMG file and its volume title now use DESKTOP_DISPLAY_NAME, so a Paradev build ships Paradev-<version>-<arch>.dmg. Unset, names stay Superset. The release keeps the Superset-<arch>.dmg alias beside the new one, since the README, docs and download button link to it. The .app bundle and the -mac.zip update files keep the Superset name. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
What & why
Fixes #8445.
connectRelaycalledhost.ensureonce per process, and a4403 forbiddenclose from the relay was retried like any other drop. A host whose access row disappeared kept reconnecting every 5 s and never recovered until restart.host.ensureagain (same 30 s to 5 min backoff as boot), then reconnects. A refusal is recorded until the nexthost.ensureresult replaces it.open, since the relay opens before it refuses.host.ensureattempt has a 30 s deadline so a stalled request can't block reconnects.How I tested it
mainit got 4403 on every reconnect for a minute and never calledhost.ensureagain. With this branch it re-registered on the first 4403 and reconnected.bun test src/tunnel/inpackages/host-service: new tests for the 4403 recovery against a local relay, the backoff sequence, and the ensure deadline. Each fails onmain.packages/host-servicesuite: 2644 pass, 0 fail.Checklist
type(scope): subject)bun run lintandbun run typecheckpass (CI fails on lint warnings too)Summary by CodeRabbit