Skip to content

Improve type safety and error logging in realtime WebSocket handler#30

Merged
skullcrushercmd merged 2 commits intomainfrom
copilot/improve-type-safety-upstreamws
Mar 4, 2026
Merged

Improve type safety and error logging in realtime WebSocket handler#30
skullcrushercmd merged 2 commits intomainfrom
copilot/improve-type-safety-upstreamws

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 4, 2026

upstreamWs and messageBuffer were typed as any/any[], bypassing type safety, and console.error was used inconsistently instead of the imported logError.

Changes

  • Type aliases: Added RealtimeMessage = string | Buffer | ArrayBuffer | Uint8Array for buffer entries
  • Typed variables: upstreamWs: NodeWebSocket | null, messageBuffer: RealtimeMessage[]
  • Removed any casts: Eliminated new (NodeWebSocket as any)(...) and all (upstreamWs as any).on(...) call sites
  • Consistent error logging: Replaced console.error with logError for upstream errors and usage update failures; usage failures now include structured context (error, userApiKey, totalTokens)
// Before
let upstreamWs: any = null;
let messageBuffer: any[] = [];
upstreamWs = new (NodeWebSocket as any)(config.url, { ... });
(upstreamWs as any).on('open', () => { ... });
updateUserTokenUsage(...).catch(err => console.error('Realtime usage update failed', err));

// After
let upstreamWs: NodeWebSocket | null = null;
let messageBuffer: RealtimeMessage[] = [];
upstreamWs = new NodeWebSocket(config.url, { ... });
upstreamWs.on('open', () => { ... });
updateUserTokenUsage(...).catch(err => logError('Realtime usage update failed', { error: err, userApiKey, totalTokens: total }));
Original prompt
Please apply the following diffs and create a pull request.
Once the PR is ready, give it a title based on the messages of the fixes being applied.

[{"message":"The upstreamWs variable is typed as 'any', which bypasses type safety. Consider creating a proper type definition for the upstream WebSocket or using NodeWebSocket type to improve type safety and maintainability.","fixFiles":[{"filePath":"apps/api/ws/realtime.ts","diff":"diff --git a/apps/api/ws/realtime.ts b/apps/api/ws/realtime.ts\n--- a/apps/api/ws/realtime.ts\n+++ b/apps/api/ws/realtime.ts\n@@ -83,7 +83,7 @@\n \n export function attachRealtimeWebSocket(app: { ws: (path: string, handler: (ws: WSWrapper, req: RequestContext) => void) => void }) {\n     app.ws('/v1/realtime', (clientWs: WSWrapper, req: RequestContext) => {\n-        let upstreamWs: any = null;\n+        let upstreamWs: NodeWebSocket | null = null;\n         let isAuthenticated = false;\n         let userId: string | null = null;\n         let userApiKey: string | null = null;\n"}]},{"message":"The messageBuffer is typed as 'any[]', which reduces type safety. Consider defining a more specific type such as 'Array<Buffer | ArrayBuffer | string>' to match the actual data types being stored.","fixFiles":[{"filePath":"apps/api/ws/realtime.ts","diff":"diff --git a/apps/api/ws/realtime.ts b/apps/api/ws/realtime.ts\n--- a/apps/api/ws/realtime.ts\n+++ b/apps/api/ws/realtime.ts\n@@ -11,6 +11,8 @@\n     model: string;\n }\n \n+type RealtimeMessage = string | Buffer | ArrayBuffer | Uint8Array;\n+\n function isOpenAIHost(urlStr?: string | null): boolean {\n     if (!urlStr) return false;\n     try {\n@@ -87,7 +89,7 @@\n         let isAuthenticated = false;\n         let userId: string | null = null;\n         let userApiKey: string | null = null;\n-        let messageBuffer: any[] = []; // Buffer messages until upstream connects\n+        let messageBuffer: RealtimeMessage[] = []; // Buffer messages until upstream connects\n \n         // 1. Authenticate Client\n         // Query param 'api-key' or Header 'Authorization' (or 'api-key')\n"}]},{"message":"Casting NodeWebSocket to 'any' to bypass type checks suggests a typing issue. Consider properly typing the WebSocket instance or using the correct constructor signature from the @types/ws package to maintain type safety.","fixFiles":[{"filePath":"apps/api/ws/realtime.ts","diff":"diff --git a/apps/api/ws/realtime.ts b/apps/api/ws/realtime.ts\n--- a/apps/api/ws/realtime.ts\n+++ b/apps/api/ws/realtime.ts\n@@ -143,23 +143,22 @@\n             }\n \n             try {\n-                // Use 'any' cast to bypass strict DOM WebSocket constructor checks\n-                upstreamWs = new (NodeWebSocket as any)(config.url, {\n+                upstreamWs = new NodeWebSocket(config.url, {\n                     headers: {\n                         'Authorization': `Bearer ${config.apiKey}`,\n                         'OpenAI-Beta': 'realtime=v1',\n                     }\n                 });\n \n-                (upstreamWs as any).on('open', () => {\n+                upstreamWs.on('open', () => {\n                     // Flush buffer\n                     for (const msg of messageBuffer) {\n-                        (upstreamWs as any)?.send(msg);\n+                        upstreamWs.send(msg);\n                     }\n                     messageBuffer = [];\n                 });\n \n-                (upstreamWs as any).on('message', async (data: any) => {\n+                upstreamWs.on('message', async (data: any) => {\n                     // Forward to client\n                     // uWS send expects string or ArrayBuffer\n                     // WebSocket data is Buffer | ArrayBuffer | Buffer[]\n@@ -195,13 +182,13 @@\n                     }\n                 });\n \n-                (upstreamWs as any).on('error', (err: any) => {\n+                upstreamWs.on('error', (err: any) => {\n                     console.error('Realtime Upstream Error:', err);\n                     clientWs.send(JSON.stringify({ type: 'error', error: { type: 'server_error', message: 'Upstream connection error.' } }));\n                     clientWs.close();\n                 });\n \n-                (upstreamWs as any).on('close', () => {\n+                upstreamWs.on('close', () => {\n                     clientWs.close();\n                 });\n \n"}]},{"message":"The error message 'Realtime usage update failed' is not informative enough. Consider including additional context such as the user ID, token count, or error details to aid in debugging production issues. Also consider using the logError function that is imported but not used here for consistent error logging.","fixFiles":[{"filePath":"apps/api/ws/realtime.ts","diff":"diff --git a/apps/api/ws/realtime.ts b/apps/api/ws/realtime.ts\n--- a/apps/api/ws/realtime.ts\n+++ b/apps/api/ws/realtime.ts\n@@ -181,7 +181,13 @@\n                                 const usage = event.response.usage;\n             ...

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

Co-authored-by: skullcrushercmd <93234024+skullcrushercmd@users.noreply.github.com>
Copilot AI changed the title [WIP] Improve type safety for upstreamWs variable Improve type safety and error logging in realtime WebSocket handler Mar 4, 2026
@skullcrushercmd skullcrushercmd marked this pull request as ready for review March 4, 2026 13:32
Copilot AI review requested due to automatic review settings March 4, 2026 13:32
@skullcrushercmd skullcrushercmd merged commit 1827a5c into main Mar 4, 2026
4 checks passed
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens TypeScript types in the realtime WebSocket proxy and aims to standardize error logging by replacing ad-hoc console.error usage with the shared logError utility.

Changes:

  • Introduces a RealtimeMessage union type and applies it to the buffered message queue.
  • Types upstreamWs as NodeWebSocket | null and removes any casts around WS construction and event handlers.
  • Replaces console.error calls with logError for upstream errors and usage-update failures.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread apps/api/ws/realtime.ts
Comment on lines +185 to +191
updateUserTokenUsage(total, userApiKey).catch(err => {
logError('Realtime usage update failed', {
error: err,
userApiKey,
totalTokens: total,
});
});
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logError is being called with a message string as the first argument and a context object as the second, but logError’s signature is logError(error, request?). As written, the context object will be treated as the (optional) request and the actual error/context won’t be captured. Consider passing a single structured object (e.g., with message and the caught error/details) as the first argument and, if available, the WS req as the second. Also avoid logging the raw API key in details—redact or hash it before including it in any logged context.

Copilot uses AI. Check for mistakes.
Comment thread apps/api/ws/realtime.ts
(upstreamWs as any).on('error', (err: any) => {
console.error('Realtime Upstream Error:', err);
upstreamWs.on('error', (err: any) => {
logError('Realtime Upstream Error', err);
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logError('Realtime Upstream Error', err) passes the message string as the error and the actual err as the request parameter, which drops the upstream error details in logs. Update this to log the actual error (and optionally include a message via a structured error object), and make sure to handle the returned promise (await it or attach a .catch) to avoid unhandled rejections.

Suggested change
logError('Realtime Upstream Error', err);
logError({ message: 'Realtime Upstream Error', error: err }).catch(() => {
// Swallow logging errors to avoid unhandled rejections in the WebSocket error handler
});

Copilot uses AI. Check for mistakes.
@skullcrushercmd skullcrushercmd deleted the copilot/improve-type-safety-upstreamws branch March 5, 2026 00:36
skullcrushercmd added a commit that referenced this pull request Apr 27, 2026
Pulls in AnyScan PRs #14 through #30:

- #14 bbe4cf2 fix: triage and fix the 11 baseline test failures
- #15 d43d05f feat(followon): stream port-scan results into host-scan tasks
- #16 0918271 feat: GraphQL endpoint and introspection discovery rules
- #17 f2eb2d7 feat: dependency-manifests path profile
- #18 feat: container-orchestration path profile
- #19 feat: build-artifacts path profile
- #20 feat: cicd-configs path profile
- #21 feat: mobile-artifacts path profile
- #22 feat: cloud-storage-listing path profile
- #23 feat: MLOps tech fingerprints
- #24 feat: modern JS framework tech fingerprints
- #25 feat: verbose stack-trace disclosure detector
- #26 feat: modern credential-token detectors (GH PAT v2 / JWT alg:none / Cloudflare / Datadog)
- #27 feat: HTTP header-policy detectors (CORS/HSTS/CSP)
- #28 e97012f9 fix(worker): reserve egress bandwidth for agentd via tc/qdisc + safety rate cap
- #29 d4986a7 feat(operator): shared shell + /app/overview page (pilot)
- #30 ef60f6f1 feat(worker): fetch inventory policy from control plane at agentd startup

Co-authored-by: skullcmd <skullcmd@anyvm.tech>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants