Skip to content

fix: add extension code for po errors from websockets#2792

Merged
SkArchon merged 3 commits intomainfrom
milinda/eng-9347-router-add-extension-error-code-for-persisted-query-not
Apr 27, 2026
Merged

fix: add extension code for po errors from websockets#2792
SkArchon merged 3 commits intomainfrom
milinda/eng-9347-router-add-extension-error-code-for-persisted-query-not

Conversation

@SkArchon
Copy link
Copy Markdown
Contributor

@SkArchon SkArchon commented Apr 23, 2026

Align websocket PO errors with normal http synchronous PO errors.

Summary by CodeRabbit

  • Bug Fixes

    • Standardized websocket error responses for unknown persisted GraphQL operations (consistent message + extension code).
    • Consistent extension codes for batch-related rejections and request cancellation to improve client-facing error signals.
  • Tests

    • Added tests covering persisted-operation safelist enforcement with enhanced logging for unknown subscription queries.

Checklist

Open Source AI Manifesto

This project follows the principles of the Open Source AI Manifesto. Please ensure your contribution aligns with its principles.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 23, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 31eced47-9077-429f-9efa-6e412de5f2a2

📥 Commits

Reviewing files that changed from the base of the PR and between c673a3a and 5ae8311.

📒 Files selected for processing (4)
  • router/core/batch.go
  • router/core/errors.go
  • router/core/graphql_prehandler.go
  • router/core/websocket.go

Walkthrough

Websocket error handling and GraphQL extension codes were standardized: persisted-operation "not found" errors now produce a structured PersistedQueryNotFound GraphQL error with an extension code. Tests were added/updated to assert the error payload and related logging. Batch and error-code constants were consolidated.

Changes

Cohort / File(s) Summary
Websocket tests
router-tests/subscriptions/websocket_test.go
Updated test expectation to assert structured PersistedQueryNotFound error payload; added test for persisted-operation safelist enforcement with LogUnknown enabled that also asserts a single "Unknown persisted operation found" log entry containing the original query text and sha256Hash.
Websocket error handling
router/core/websocket.go
writeErrorMessage now recognizes PersistentOperationNotFoundError via errors.As and emits a single-element GraphQL error with message = "PersistedQueryNotFound" and extensions.code = ExtCodeErrPersistedQueryNotFound; other errors preserved. JSON payload marshaling simplified to always marshal that single error.
GraphQL error codes
router/core/errors.go
Introduced shared ExtCodeErr* constants (including ExtCodeErrPersistedQueryNotFound, ExtCodeErrErrorRequestCanceled, ExtCodeErrBatchSizeExceeded, ExtCodeErrBatchSubscriptionsUnsupported) and updated writeOperationError to use these constants for specific cases.
Batch handling
router/core/batch.go
Replaced inline/old batch extension code usage with ExtCodeErrBatchSizeExceeded; removed a local constants block for batch-related extension codes.
Operation prehandler
router/core/graphql_prehandler.go
Batch-subscription rejection now sets httpGraphqlError.extensionCode to ExtCodeErrBatchSubscriptionsUnsupported (constant name alignment).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main purpose of the pull request: adding extension codes for persisted operation errors in websocket responses.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.11.4)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"


Comment @coderabbitai help to get the list of available commands and usage tips.

@SkArchon SkArchon changed the title fix: Add extension code for po errors from websockets fix: add extension code for po errors from websockets Apr 23, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 23, 2026

Router image scan passed

✅ No security vulnerabilities found in image:

ghcr.io/wundergraph/cosmo/router:sha-5fdd9d1459a44bc79a89c55550e220c3a42f9d5a

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 23, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.64%. Comparing base (fdd035b) to head (98bed3f).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #2792       +/-   ##
===========================================
+ Coverage   46.28%   65.64%   +19.36%     
===========================================
  Files        1045      254      -791     
  Lines      139773    26457   -113316     
  Branches     8768        0     -8768     
===========================================
- Hits        64695    17368    -47327     
+ Misses      73326     7687    -65639     
+ Partials     1752     1402      -350     
Files with missing lines Coverage Δ
router/core/batch.go 80.31% <100.00%> (ø)
router/core/errors.go 80.64% <100.00%> (ø)
router/core/graphql_prehandler.go 84.63% <100.00%> (ø)
router/core/websocket.go 76.51% <100.00%> (+0.57%) ⬆️

... and 806 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread router/core/websocket.go
@SkArchon SkArchon enabled auto-merge (squash) April 27, 2026 09:25
@SkArchon SkArchon merged commit b1c17b0 into main Apr 27, 2026
37 checks passed
@SkArchon SkArchon deleted the milinda/eng-9347-router-add-extension-error-code-for-persisted-query-not branch April 27, 2026 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants