fix: ensure provider-proxy does not crash when socket is closed during cert verification

[stalniy]

Why

provider proxy crashes with this error:

Summary by CodeRabbit

• New Features

  • No user-facing features added.

• Bug Fixes

  • Improves reliability when establishing provider WebSocket connections.
  • Ensures certificate verification status is accurately reflected.
  • Reduces risks of stale state or intermittent errors during verification.

• Refactor

  • Streamlines connection state management with a single cached reference.
  • Uses consistent existence checks to avoid edge-case failures.
  • Reduces redundant lookups, improving stability and maintainability.

[coderabbitai]

Walkthrough

Refactors provider WebSocket creation to cache the socket details (wsDetails) and update verification state via this reference. Replaces direct map indexing with wsDetails, uses Object.hasOwn for existence checks, updates verification on TLS validation outcomes, and returns wsDetails at the end.

Changes

Cohort / File(s)	Summary
Provider WebSocket state handling apps/provider-proxy/src/services/WebsocketServer.ts	Cached wsDetails introduced; state updates consolidated to wsDetails; replaced direct indexing into openProviderSockets; switched existence checks to Object.hasOwn; adjusted verification updates on success/failure and finalization; return path now returns wsDetails.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Client
  participant Server as WebsocketServer
  participant Map as openProviderSockets
  participant TLS as TLS Validator

  Client->>Server: createProviderSocket(options)
  Server->>Map: init entry for options.wsId
  Server->>Server: wsDetails = Map[wsId]
  Server->>TLS: validate CA/certificate
  alt Validation success
    TLS-->>Server: OK
    Server->>Server: wsDetails.verification = "finished"
  else Validation failure
    TLS-->>Server: Error
    Server->>Server: wsDetails.verification = "failed"
  end
  Server->>Server: if Object.hasOwn(Map, wsId)
  alt Present
    Server->>Server: wsDetails.verification = "finished" (finalization path)
  else Removed
    Server->>Server: skip update
  end
  Server-->>Client: return wsDetails

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• fix: improves observability of provider websocket #1751 — Touches the same createProviderSocket and certificate-validation paths, adding logging/tracing around these flows.

Suggested reviewers

• baktun14
• ygrishajev

Poem

I buffered a socket, snug in its nest,
Cached little details to give state a rest.
HasOwn I nibble, indexes I shun—
Verification flips when checks are done.
With whiskers twitching, I hop to say:
wsDetails returns, clean as hay. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title concisely describes the main change of preventing a crash in the provider-proxy when a socket is closed during certificate verification, aligning directly with the pull request’s purpose and avoiding extraneous detail.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/provider-websocket-crash

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0280541 and 3d9c4fb.

📒 Files selected for processing (1)

• apps/provider-proxy/src/services/WebsocketServer.ts (5 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

Never use type any or cast to type any. Always define the proper TypeScript types.

Files:

• apps/provider-proxy/src/services/WebsocketServer.ts

**/*.{js,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code

Files:

• apps/provider-proxy/src/services/WebsocketServer.ts

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: test-build

🔇 Additional comments (3)

apps/provider-proxy/src/services/WebsocketServer.ts (3)

265-265: LGTM! Smart caching pattern to prevent race condition crashes.

Caching the socket details reference immediately after storing in the map ensures a stable reference throughout the async certificate validation flow. This prevents crashes when the socket is removed from the map (e.g., client disconnect) while certificate verification is in progress.

---

267-345: Excellent race condition handling with cached reference and existence check.

The cached wsDetails reference enables safe verification status updates even after the socket is removed from the map:

• Lines 274, 311, 322: Setting wsDetails.verification works safely regardless of map state
• Line 321: Object.hasOwn() check prevents emitting "verified" event on already-closed sockets while still allowing verification status updates for the close handler (line 434)

The use of Object.hasOwn() is the correct choice over alternatives like in operator or truthiness checks.

---

347-347: LGTM! Consistent return of cached reference.

Returning wsDetails instead of performing another map lookup maintains consistency with the caching pattern and ensures the caller receives a valid reference even if the socket was concurrently removed from the map. The close handler (line 434) correctly accesses verification status via this cached reference passed as a parameter.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

❌ Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 45.59%. Comparing base (0280541) to head (3d9c4fb).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...pps/provider-proxy/src/services/WebsocketServer.ts	83.33%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2033      +/-   ##
==========================================
- Coverage   45.95%   45.59%   -0.37%     
==========================================
  Files        1014     1004      -10     
  Lines       28633    28285     -348     
  Branches     7513     7471      -42     
==========================================
- Hits        13158    12896     -262     
+ Misses      15192    15115      -77     
+ Partials      283      274       -9     

Flag	Coverage Δ		*Carryforward flag
api	82.24% <ø> (ø)		Carriedforward from 0280541
deploy-web	23.87% <ø> (ø)		Carriedforward from 0280541
log-collector	?
notifications	88.16% <ø> (ø)		Carriedforward from 0280541
provider-console	81.48% <ø> (ø)		Carriedforward from 0280541
provider-proxy	85.00% <83.33%> (+0.02%)	⬆️

*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines	Coverage Δ
...pps/provider-proxy/src/services/WebsocketServer.ts	85.78% <83.33%> (+0.07%)	⬆️

... and 10 files with indirect coverage changes

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.