fix(websocket): throw on ExecutionContext.switchToRpc()#93
Conversation
Closes FOSSFORGE#89 WebSocket guards and exception filters previously returned a fake RPC context that mapped client/data into RPC fields. This is semantically incorrect — we're not in an RPC context — and matches the issue reporter's expectation that switchToRpc should throw the same way switchToHttp already does.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/websocket/middleware/filters/exception-filter-executor.spec.ts`:
- Around line 146-162: The test currently relies on an assertion inside
RpcCheckFilter.catch which can be swallowed by ExceptionFilterExecutor.catch;
instead have the filter call host.switchToRpc() so it throws naturally and
assert on executor.catch itself: update RpcCheckFilter.catch to simply invoke
host.switchToRpc() (no expect inside the filter), then call await
expect(executor.catch(new Error('Test'), host)).rejects.toThrow('RPC context not
available in WebSocket'); keep references to ExceptionFilterExecutor.catch
(executor.catch), RpcCheckFilter.catch, TestGateway and createHost to locate the
code.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 717bd777-d903-4ded-bea0-c74adf5857b1
📒 Files selected for processing (4)
src/websocket/middleware/filters/exception-filter-executor.spec.tssrc/websocket/middleware/filters/exception-filter-executor.tssrc/websocket/middleware/guards/guard-executor.spec.tssrc/websocket/middleware/guards/guard-executor.ts
|
@mvanhorn LGTM, Thank you |
What changed
src/websocket/middleware/guards/guard-executor.tsandsrc/websocket/middleware/filters/exception-filter-executor.ts: replaced theswitchToRpcadapter (which previously returned a fake RPC context wrapping the WebSocket client and data) with a throwing implementation that mirrors the existingswitchToHttpbehavior:Added two unit tests — one in
guard-executor.spec.ts, one inexception-filter-executor.spec.ts— that build a guard / filter callingcontext.switchToRpc()and assert the throw.Closes #89
Why
Issue #89 calls out that the existing
switchToRpcreturns a fake context whosegetContext()andgetData()map WebSocket fields into RPC slots. That's semantically wrong — the execution context is never an RPC context. The expected behavior, per the NestJS reference linked in the issue (packages/core/helpers/execution-context-host.ts), is that the wrong-context switches throw. The codebase already does this forswitchToHttp; this PR makesswitchToRpcsymmetric.Verification
npx jest src/websocket/middleware/guards/guard-executor.spec.ts src/websocket/middleware/filters/exception-filter-executor.spec.tsruns 33/33 green, including the two new tests. The new tests use the same gateway construction and execution invocation pattern as the existingswitchToWstests in those files.Summary by CodeRabbit
Bug Fixes
Tests