fix: use correct WebSocket transport and improve error message in PerplexityBot - #1079
fix: use correct WebSocket transport and improve error message in PerplexityBot#1079octo-patch wants to merge 1 commit into
Conversation
…plexityBot (fixes ai-shifu#925) The WebSocket URL incorrectly used `transport=polling` instead of `transport=websocket`, causing the WebSocket upgrade to fail. When the connection failed, `reject(event)` was called with a raw DOM Event object, producing the unhelpful "[object Event]" error message. - Change WebSocket URL transport from `polling` to `websocket` - Replace `reject(event)` with a localized error message using `i18n.global.t("error.failedConnectUrl")`, consistent with MOSSBot Co-Authored-By: Octopus <liyuan851277048@icloud.com>
📝 WalkthroughWalkthroughModified error handling and transport protocol in the Perplexity bot by switching from polling to WebSocket transport and replacing raw error rejection with localized error messages using the i18n system. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the PerplexityBot to use the websocket transport for its connection and implements localized error messages using i18n. A security concern was raised regarding the exposure of sensitive query parameters, such as session IDs, in the error message's URL; a suggestion was provided to strip these parameters before display to prevent accidental information exposure.
| reject(event); | ||
| reject( | ||
| i18n.global.t("error.failedConnectUrl", { | ||
| url: event.target?.url ?? "wss://www.perplexity.ai/socket.io/", |
There was a problem hiding this comment.
The WebSocket URL contains sensitive query parameters, including the session ID (sid) and a timestamp (t). Displaying the full URL in the error message could lead to accidental exposure of these tokens if a user shares a screenshot of the error. It is safer to strip the query parameters and only display the base URL in the UI.
| url: event.target?.url ?? "wss://www.perplexity.ai/socket.io/", | |
| url: event.target?.url?.split("?")[0] ?? "wss://www.perplexity.ai/socket.io/", |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/bots/PerplexityBot.js (1)
310-318: Localized error rejection looks good; minor convention nit.The handler now rejects with a human-readable, translated message and includes a sensible fallback URL when
event.target?.urlis unavailable, matching the MOSSBot pattern (src/bots/MOSSBot.js:126-134). This resolves the "[object Event]" symptom from issue#925.Optional nit: like MOSSBot, this rejects with a raw string rather than an
Errorinstance, which can lose stack-trace context for any upstreamcatchthat expectserror.message. If callers don't depend on that, feel free to ignore — keeping consistency with MOSSBot has its own value. If you'd like to harden it without changing the displayed text:♻️ Optional: wrap in Error for consistent error shape
wsp.onError.addListener((event) => { wsp.removeAllListeners(); wsp.close(); reject( - i18n.global.t("error.failedConnectUrl", { - url: event.target?.url ?? "wss://www.perplexity.ai/socket.io/", - }), + new Error( + i18n.global.t("error.failedConnectUrl", { + url: event.target?.url ?? "wss://www.perplexity.ai/socket.io/", + }), + ), ); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bots/PerplexityBot.js` around lines 310 - 318, The onError listener in PerplexityBot.js currently calls reject(...) with a localized string; change it to reject(new Error(...)) so the rejection is an Error instance (preserving stack and shape) while still using the same i18n.global.t("error.failedConnectUrl", { url: ... }) message and the same fallback URL; update the wsp.onError.addListener callback to construct an Error with that translated message before calling reject so behavior matches MOSSBot's convention and retains the human-readable localized text.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/bots/PerplexityBot.js`:
- Around line 310-318: The onError listener in PerplexityBot.js currently calls
reject(...) with a localized string; change it to reject(new Error(...)) so the
rejection is an Error instance (preserving stack and shape) while still using
the same i18n.global.t("error.failedConnectUrl", { url: ... }) message and the
same fallback URL; update the wsp.onError.addListener callback to construct an
Error with that translated message before calling reject so behavior matches
MOSSBot's convention and retains the human-readable localized text.
1 participant
Fixes #925
Problem
The Perplexity bot was showing
[object Event]as the error message when the WebSocket connection failed. There were two bugs:The WebSocket URL used
transport=pollinginstead of the correcttransport=websocketfor a WebSocket connection. In socket.io's upgrade flow, the WebSocket endpoint must specifytransport=websocket— usingtransport=pollingcauses the server to reject or mishandle the upgrade, leading to connection failure.When the connection failed,
reject(event)passed a raw DOMEventobject to the rejection handler. When converted to a string viatoString(), it produces"[object Event]"— an unhelpful error message shown to users.Solution
transport=pollingtotransport=websocketreject(event)with a localized error message usingi18n.global.t("error.failedConnectUrl"), consistent with the pattern already used inMOSSBot.jsTesting
The fix aligns the WebSocket transport with what socket.io expects for a WebSocket upgrade, and ensures error messages are human-readable when the connection fails.
Summary by CodeRabbit