Skip to content

Conversation

@marcodejongh
Copy link
Owner

Summary

This PR enhances error handling for GraphQL WebSocket connections and improves error reporting by adding better diagnostics for origin/CORS issues and filtering out non-actionable connection errors from error tracking.

Key Changes

GraphQL Client Improvements (graphql-client.ts)

  • Added origin error detection: Created isOriginError() utility and ORIGIN_ERROR_PATTERNS to identify browser-specific CORS/origin validation failures
  • Wrapped WebSocket class: Implemented createWrappedWebSocket() that extends the native WebSocket with early error handlers to catch connection failures before graphql-ws sets up its handlers
    • Captures and logs early error and close events with context
    • Provides special handling for origin-related errors with helpful diagnostic messages
  • Smart retry logic: Updated shouldRetry() to skip retries on origin/CORS errors since these won't resolve without configuration changes
  • Enhanced error logging: Improved error and close event handlers to provide actionable context about connection failures, including potential causes (browser privacy settings, network issues, CORS configuration)

Error Tracking Improvements (instrumentation-client.ts)

  • Added error filtering: Created shouldFilterError() utility and FILTERED_ERROR_PATTERNS to identify expected connection errors
  • Sentry beforeSend hook: Implemented filtering logic to prevent non-actionable errors (WebSocket origin errors, network errors, connection lifecycle events) from cluttering error tracking
  • Maintains console logging for debugging while reducing noise in error tracking

Implementation Details

  • Origin error detection uses pattern matching on error messages to identify browser-specific CORS failures (e.g., "invalid origin", "origin not allowed")
  • The wrapped WebSocket removes early handlers after first fire to avoid duplicate handling by graphql-ws
  • Error filtering is applied at the Sentry level to maintain visibility in development while reducing production noise
  • All changes are backward compatible and don't affect the public API

https://claude.ai/code/session_014RG7hQPmmTu3acu8W2ApKN

@vercel
Copy link

vercel bot commented Feb 1, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
boardsesh Building Building Preview, Comment Feb 1, 2026 8:27am

Request Review

@claude
Copy link

claude bot commented Feb 1, 2026

Claude Review

Ready to merge - Minor issues noted below, but nothing blocking.

Issues

  1. Duplicate pattern definitions - ORIGIN_ERROR_PATTERNS in graphql-client.ts:19-24 and FILTERED_ERROR_PATTERNS in instrumentation-client.ts:17-27 overlap significantly. Consider extracting shared patterns to a common module to avoid drift.

  2. Overly broad error filtering - instrumentation-client.ts:22-26 filters generic patterns like "failed to fetch", "websocket is closed", and "connection closed" which may suppress legitimate errors (e.g., API endpoint failures, unexpected disconnections). Consider adding more specific patterns or context checking.

  3. No tests - No unit tests for the new error detection utilities (isOriginError, shouldFilterError) or the wrapped WebSocket class. These are testable pure functions that would benefit from coverage for edge cases.

Documentation

The PR adds error handling improvements to the WebSocket client documented in docs/websocket-implementation.md. The existing documentation at lines 99-156 already covers origin validation and reconnection, so no doc update is strictly required, though you could consider adding a note about the enhanced error diagnostics.

@claude
Copy link

claude bot commented Feb 1, 2026

Claude Review

Ready to merge - Well-structured error handling improvements with good test coverage.

Minor Issues

  1. Overly broad error filtering - websocket-errors.ts:36-38: The pattern 'websocket connection to' will match ANY WebSocket connection error message, not just origin-related ones. This could suppress legitimate errors that should be reported to Sentry.

  2. Missing null/undefined check - graphql-client.ts:39: errorEvent.message could be undefined for some error events. The code handles this with || 'Unknown WebSocket error', but the isOriginError function would still receive an empty string if errorEvent.message is an empty string (which is falsy but passes through the || operator correctly, so this is actually fine on second thought).

  3. Test coverage gap - websocket-errors.test.ts: No test for empty string or undefined input to isOriginError() / shouldFilterFromSentry(). While TypeScript enforces string type, runtime edge cases could occur.

Documentation

The changes add error handling features to the WebSocket client documented in docs/websocket-implementation.md. The doc references the graphql-client.ts file but doesn't document the new error filtering behavior. Consider adding a brief section on error handling/filtering if this becomes important for debugging production issues.

Add proper error handling for WebSocket connection failures, particularly
the "invalid origin" error that occurs on some browsers (especially Safari/
WebKit on iOS via DuckDuckGo Mobile). This error can be caused by:
- Browser privacy/tracking protection blocking WebSocket connections
- Network issues during WebSocket handshake
- CORS configuration issues

Changes:
- Add wrapped WebSocket implementation with early error handlers to catch
  connection failures before graphql-ws sets up its handlers
- Skip retry attempts for origin/CORS related errors since they won't
  resolve without configuration changes
- Add Sentry beforeSend filter to prevent these known connection errors
  from cluttering error tracking
- Improve logging with more context for origin-related failures

https://claude.ai/code/session_014RG7hQPmmTu3acu8W2ApKN
Address code review feedback:
- Extract shared error patterns to packages/web/app/lib/websocket-errors.ts
  to avoid duplication between graphql-client.ts and instrumentation-client.ts
- Make error filtering more specific: only filter WebSocket-specific errors,
  not generic network errors like "failed to fetch" that could indicate
  legitimate API failures
- Add unit tests for error detection utilities (isOriginError,
  isWebSocketLifecycleError, shouldFilterFromSentry)

https://claude.ai/code/session_014RG7hQPmmTu3acu8W2ApKN
Document the error handling and filtering behavior for WebSocket connections,
including which errors are filtered from Sentry and why.

https://claude.ai/code/session_014RG7hQPmmTu3acu8W2ApKN
@marcodejongh marcodejongh force-pushed the claude/fix-invalid-origin-error-4rHXy branch from 88a0e7f to 45c7618 Compare February 1, 2026 08:27
@claude
Copy link

claude bot commented Feb 1, 2026

Claude Review

Ready to merge - No significant issues found.

The PR adds well-structured WebSocket error handling with proper test coverage. The documentation has been updated appropriately in docs/websocket-implementation.md. The test file covers happy paths and edge cases (null/undefined handling, case insensitivity, various error patterns). The changes are focused and don't over-engineer the solution.

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