feat(websocket): add real-time WebSocket support via gorilla/websocket#26
Merged
GRACENOBLE merged 2 commits intoJun 15, 2026
Merged
Conversation
Implements issue #21 across all three layers: **Backend:** gorilla/websocket Hub with channel-based fan-out, GET /ws endpoint authenticated via Firebase token query param, hub wired as a long-lived goroutine in main.go with graceful shutdown. Handler struct now carries verifier and hub; RegisterRoutes signature simplified. **Web:** useWebSocket hook with typed WsEnvelope, exponential-backoff reconnection (capped at 30s), Firebase token as ?token= query param, and cleanup on unmount. Ref mutations moved to useLayoutEffect per React 19 react-hooks/refs rule. **Mobile:** OkHttp WebSocketManager with injectable factory and reconnect scheduler for testability, WebSocketViewModel exposing StateFlow<WsState>, kotlinx.serialization for JSON parsing. Hub.Publish() left as the integration point for issue #18 (Asynq + Redis Streams).
7 tasks
github-actions
Bot
added
area: backend
|
Warning Review limit reached
More reviews will be available in 35 minutes and 38 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughAdds end-to-end WebSocket support: a Go ChangesBackend Go WebSocket infrastructure and wiring
Android WebSocket client (OkHttp + ViewModel)
React useWebSocket hook
Sequence Diagram(s)sequenceDiagram
participant Client as Browser/Mobile Client
participant WsHandler
participant Firebase as Firebase Token Verifier
participant Hub
participant WsClient as ws.Client
Client->>WsHandler: GET /ws?token=<firebase_id_token>
WsHandler->>Firebase: VerifyIDToken(token)
alt token invalid or missing
Firebase-->>WsHandler: error
WsHandler-->>Client: 401 Unauthorized
else token valid
Firebase-->>WsHandler: ok
WsHandler->>WsHandler: websocket.Upgrader.Upgrade(conn)
WsHandler->>WsClient: NewClient(hub, conn) → hub.Register
WsHandler->>WsClient: go WritePump()
WsHandler->>WsClient: go ReadPump()
Note over Hub,WsClient: Later: worker publishes event
Hub->>Hub: Publish("job.completed", payload) → broadcast channel
Hub->>WsClient: non-blocking send to client.Send
WsClient->>Client: WebSocket text frame
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/docs/routing.md (1)
33-34:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate stale signature text to match current wiring.
The prose still says
NewServer(app *bootstrap.App) *http.Serverand references passingverifiertoRegisterRoutes, but the shown code now usesNewServer(app *bootstrap.App, hub *ws.Hub)andRegisterRoutes(rps, burst, sentryDSN).Suggested fix
-`internal/server/server.go` contains `NewServer(app *bootstrap.App) *http.Server` — wiring only, no logic. +`internal/server/server.go` contains `NewServer(app *bootstrap.App, hub *ws.Hub) *http.Server` — wiring only, no logic. @@ -`verifier` is a `usecase.FirebaseTokenVerifier`; pass `nil` to skip Firebase auth (development only — see [auth](auth.md)). +Firebase verification for `/api/v1` routes is derived from `h.verifier` on `Handler`; when `h.verifier == nil`, Firebase middleware is skipped (development only — see [auth](auth.md)).Also applies to: 53-53
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/docs/routing.md` around lines 33 - 34, Update the documentation in backend/docs/routing.md to reflect the current function signatures. The `NewServer` function signature should be updated to include the `hub *ws.Hub` parameter (currently shown as only taking `app *bootstrap.App`). The `RegisterRoutes` function parameters should be updated to reflect that it now accepts `rps, burst, sentryDSN` instead of `verifier`. These corrections are needed at both line 33-34 (anchor) and line 53 (sibling) where the same outdated signatures appear.backend/cmd/api/main.go (1)
3-16:⚠️ Potential issue | 🟠 MajorAdd
godotenv/autoloadblank import incmd/apientrypoint.
backend/cmd/api/main.gois missing_ "github.com/joho/godotenv/autoload". Per coding guidelines, environment variables must be loaded automatically via this blank import in allcmdentrypoints without explicit calls.Suggested fix
import ( "context" "fmt" "log/slog" "net/http" "os" "os/signal" "syscall" "time" + _ "github.com/joho/godotenv/autoload" + "backend/internal/bootstrap" "backend/internal/infrastructure/ws" "backend/internal/server" )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/cmd/api/main.go` around lines 3 - 16, The import section in backend/cmd/api/main.go is missing the required blank import for godotenv/autoload. Add the line _ "github.com/joho/godotenv/autoload" to the import block to ensure environment variables are automatically loaded on startup according to the project's coding guidelines. This blank import should be placed with the other third-party imports in the import statement that begins at line 3.Source: Coding guidelines
🧹 Nitpick comments (2)
backend/internal/infrastructure/ws/hub_test.go (2)
121-129: StrengthenTestHub_Publishto assert envelope contract, not only non-empty bytes.The test currently validates only that bytes are received, not that they form a valid message. Unmarshal
gotintoEnvelopeand assertType == "job.completed"plus the payload contains the expected fields (e.g.,"id": "123").🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/internal/infrastructure/ws/hub_test.go` around lines 121 - 129, The TestHub_Publish test currently only validates that the message is non-empty but does not verify the actual message structure or contents. Replace the non-empty check on got with proper unmarshaling of got into an Envelope struct, then assert that the Type field equals "job.completed" and that the payload contains the expected fields (such as "id": "123"). This ensures the test validates the full message contract rather than just checking for non-empty bytes.
20-21: Replace fixed sleeps with deterministic synchronization in hub tests.These tests rely on hardcoded sleeps (5–10ms) to allow the
hub.Run()goroutine to process channel operations before assertions. This creates timing-sensitive tests that may fail if the hub's processing time changes. Instead, use observable state signals—such as a sync channel handshake, sync.WaitGroup, or polling with a timeout—to ensure the goroutine has completed the operation before proceeding.Applies to lines 20, 41, 44, 64, 67, 89, and 119.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/internal/infrastructure/ws/hub_test.go` around lines 20 - 21, Replace the hardcoded time.Sleep calls in the hub tests with deterministic synchronization mechanisms. Instead of relying on fixed delays to allow the hub.Run() goroutine to process operations, use observable state signals such as a sync channel handshake, sync.WaitGroup, or a polling loop with timeout to explicitly wait until the goroutine has completed the operation before proceeding with assertions. Apply this pattern to all occurrences of time.Sleep throughout the test file to eliminate timing-sensitive test behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/docs/websocket.md`:
- Line 65: The markdown file has unlabeled fenced code blocks that trigger
markdownlint error MD040, requiring explicit language specifiers. At line 65,
add the language identifier `text` to the opening fence of the code block
showing the goroutine communication diagram (the block starting with `caller
goroutine`). At line 98, add the language identifier `text` to the opening fence
of the code block showing the GET request example (the block starting with `GET
/ws?token`). Both changes involve changing the bare triple backticks to triple
backticks followed by the language name to satisfy the linting rules.
In `@backend/internal/infrastructure/ws/client.go`:
- Line 45: Remove the //nolint:errcheck directives from the websocket I/O
operations (SetReadDeadline, SetWriteDeadline, WriteMessage, and Write
operations) and properly handle their returned errors by returning them up the
stack instead of suppressing them. This aligns with the error-handling guideline
for backend/internal code and makes the error handling consistent with other
operations in the same file (such as the SetReadDeadline call that already
returns its error and the WriteMessage operation that already handles errors
correctly).
- Around line 40-43: The defer block in the `ReadPump` function sends on the
unbuffered `Unregister` channel without checking if `Hub.Run` has already
exited, causing a deadlock during shutdown because the channel will have no
receiver. Use a non-blocking send with a select statement and default case when
sending on `c.hub.Unregister` to prevent blocking, ensuring that
`c.conn.Close()` always executes to avoid goroutine leaks. Additionally, remove
all `//nolint:errcheck` directives throughout the file (which suppress I/O
operation errors) and instead properly handle errors by either logging them,
returning them to the caller, or explicitly documenting why they can be safely
ignored in a way that aligns with returning errors up the stack rather than
swallowing them.
In `@backend/internal/infrastructure/ws/hub.go`:
- Around line 61-72: The Publish method in the Hub type performs a blocking send
on the h.broadcast channel that can indefinitely block when the buffer is full,
causing worker stalls and backpressure issues. Replace the blocking channel send
(h.broadcast <- env) with a non-blocking send using a select statement with a
default case that handles the full channel scenario by returning an appropriate
error or dropping the message with logging/metrics.
In `@backend/internal/transport/handlers/ws_handler.go`:
- Around line 12-17: The `upgrader` variable's `CheckOrigin` function currently
returns `true` for all origins, creating a cross-site WebSocket hijacking
vulnerability. Replace the permissive `CheckOrigin` function with
environment-aware logic that returns `true` only in local development
environments (based on environment configuration) and restricts incoming
WebSocket requests to a whitelist of known, trusted origins in production.
Extract the allowed origins list from configuration and validate that the
request's origin header matches one of the allowed origins before granting
access.
In `@mobile/app/src/main/java/com/company/template/websocket/WebSocketManager.kt`:
- Around line 46-51: The connect() method opens a new socket without closing any
existing connection, which causes connection leaks and duplicate message streams
when called repeatedly. Before calling openSocket() in the connect(token:
String?) method, ensure any existing socket connection is properly closed by
checking if an active socket exists and invoking the appropriate cleanup or
disconnection method on it first.
In `@web/docs/websocket.md`:
- Around line 62-64: The fenced code block containing the delay calculation
formula lacks a language identifier, which violates markdownlint rule MD040. Add
a language identifier to the opening fence marks of the code block that contains
"delay = min(1000ms × 2^retryCount, 30 000ms)" by changing the opening ``` to
```txt (or another appropriate language identifier).
In `@web/lib/useWebSocket.ts`:
- Around line 52-53: The wsUrl construction logic at line 52 in useWebSocket.ts
unconditionally appends the token using a question mark, which creates malformed
URLs when the input url already contains query parameters. Fix this by checking
if the url already contains a query string (look for the presence of `?`), and
if it does, append the token parameter using `&` instead of `?`. Additionally,
add a regression test to verify that the token parameter is correctly appended
to URLs that already include existing query parameters.
---
Outside diff comments:
In `@backend/cmd/api/main.go`:
- Around line 3-16: The import section in backend/cmd/api/main.go is missing the
required blank import for godotenv/autoload. Add the line _
"github.com/joho/godotenv/autoload" to the import block to ensure environment
variables are automatically loaded on startup according to the project's coding
guidelines. This blank import should be placed with the other third-party
imports in the import statement that begins at line 3.
In `@backend/docs/routing.md`:
- Around line 33-34: Update the documentation in backend/docs/routing.md to
reflect the current function signatures. The `NewServer` function signature
should be updated to include the `hub *ws.Hub` parameter (currently shown as
only taking `app *bootstrap.App`). The `RegisterRoutes` function parameters
should be updated to reflect that it now accepts `rps, burst, sentryDSN` instead
of `verifier`. These corrections are needed at both line 33-34 (anchor) and line
53 (sibling) where the same outdated signatures appear.
---
Nitpick comments:
In `@backend/internal/infrastructure/ws/hub_test.go`:
- Around line 121-129: The TestHub_Publish test currently only validates that
the message is non-empty but does not verify the actual message structure or
contents. Replace the non-empty check on got with proper unmarshaling of got
into an Envelope struct, then assert that the Type field equals "job.completed"
and that the payload contains the expected fields (such as "id": "123"). This
ensures the test validates the full message contract rather than just checking
for non-empty bytes.
- Around line 20-21: Replace the hardcoded time.Sleep calls in the hub tests
with deterministic synchronization mechanisms. Instead of relying on fixed
delays to allow the hub.Run() goroutine to process operations, use observable
state signals such as a sync channel handshake, sync.WaitGroup, or a polling
loop with timeout to explicitly wait until the goroutine has completed the
operation before proceeding with assertions. Apply this pattern to all
occurrences of time.Sleep throughout the test file to eliminate timing-sensitive
test behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5ecbb672-f9d7-434c-bc16-b2d5f21e5930
⛔ Files ignored due to path filters (1)
backend/go.sumis excluded by!**/*.sum
📒 Files selected for processing (29)
backend/cmd/api/main.gobackend/docs/_index.mdbackend/docs/routing.mdbackend/docs/swagger/docs.gobackend/docs/swagger/swagger.jsonbackend/docs/swagger/swagger.yamlbackend/docs/websocket.mdbackend/go.modbackend/internal/infrastructure/ws/client.gobackend/internal/infrastructure/ws/hub.gobackend/internal/infrastructure/ws/hub_test.gobackend/internal/infrastructure/ws/message.gobackend/internal/server/server.gobackend/internal/transport/handlers/handler.gobackend/internal/transport/handlers/health_handler_test.gobackend/internal/transport/handlers/routes.gobackend/internal/transport/handlers/ws_handler.gomobile/app/build.gradle.ktsmobile/app/src/main/java/com/company/template/websocket/WebSocketManager.ktmobile/app/src/main/java/com/company/template/websocket/WebSocketViewModel.ktmobile/app/src/main/java/com/company/template/websocket/WsEnvelope.ktmobile/app/src/test/java/com/company/template/websocket/FakeOkHttp.ktmobile/app/src/test/java/com/company/template/websocket/WebSocketManagerTest.ktmobile/app/src/test/java/com/company/template/websocket/WebSocketViewModelTest.ktmobile/gradle/libs.versions.tomlweb/docs/_index.mdweb/docs/websocket.mdweb/lib/useWebSocket.test.tsweb/lib/useWebSocket.ts
- hub: make Publish non-blocking (select/default) to prevent worker stall - client: use non-blocking Unregister send to prevent goroutine leak on shutdown - client: replace //nolint:errcheck with explicit error checks throughout ReadPump/WritePump - ws_handler: restrict CheckOrigin to BLUEPRINT_WS_ALLOWED_ORIGIN in non-debug mode - WebSocketManager: close existing socket before opening on repeated connect() calls - useWebSocket: handle pre-existing query params when appending ?token= (use & separator) - tests: add regression test for URL with existing query params - docs: add language identifier to unlabeled fenced code blocks (markdownlint MD040) - .env.example: document BLUEPRINT_WS_ALLOWED_ORIGIN Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
GRACENOBLE
deleted the
21-feat-add-websocket-support-via-gorilla-websocket
branch
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #21
Summary
gorilla/websocketHub with channel-based fan-out;GET /wsendpoint with Firebase token auth via?token=query param; Hub started as a long-lived goroutine inmain.gowith context-based graceful shutdown;Handlerstruct updated to carryverifierandhubuseWebSockethook (lib/useWebSocket.ts) with typedWsEnvelope, exponential-backoff reconnection capped at 30 s, Firebase token appended as?token=, cleanup on unmountWebSocketManager(OkHttp with injectableWebSocketFactory+reconnectScheduler) andWebSocketViewModelexposingStateFlow<WsState>;kotlinx.serializationfor JSON;WsEnvelopemirrors the backend envelope typeHub.Publish(type, payload)is the integration point for feat: integrate Asynq + Redis Streams for background jobs and event-driven messaging #18 (Asynq + Redis Streams) — no Redis consumer in this PRbackend/docs/websocket.md,web/docs/websocket.md;backend/docs/routing.mdupdated to reflect signature changesTest plan
go test ./internal/infrastructure/ws/...— 5 hub tests (register, unregister, cancel, 10 concurrent clients, Publish)go test ./internal/transport/...— existing handler/middleware tests unaffectedpnpm test— 8useWebSockettests (URL, token, connect/disconnect, messages, malformed JSON, backoff, maxRetries, unmount cleanup, send)./gradlew test— 8WebSocketManagerTest+ 6WebSocketViewModelTestmake docker-run && make watch, connect withwebsocat ws://localhost:8080/ws(no token, dev mode) — should upgrade and receive anyhub.PublishcallsGET /wswithout?token=returns 401; with valid Firebase token upgrades successfullySummary by CodeRabbit
Release Notes
GET /wsAPI endpoint with token-based authentication