feat(router) Add support for per message deflate extension to websockets #2424
Conversation
dbinnersley
requested review from
Noroth,
StarpTech,
devsergiy,
endigma and
jensneuse
as code owners
WalkthroughAdds permessage-deflate WebSocket compression: new configuration, upgrade negotiation, compression-aware connection wrapper with compressed read/write paths, and tests + test helpers exercising compression negotiation and payload exchange. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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 |
|
@coderabbitai summary |
✅ Actions performedSummary regeneration triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
router/core/websocket.go (1)
285-302: Consider handlingflate.NewWritererror.The error from
flate.NewWriteris currently ignored. While the compression level is validated during initialization, explicitly handling this error would be more defensive.🔎 Proposed defensive error handling
func (c *wsConnectionWrapper) writeCompressed(data []byte) error { var buf bytes.Buffer writer := wsflate.NewWriter(&buf, func(w io.Writer) wsflate.Compressor { - fw, _ := flate.NewWriter(w, c.compressionLevel) + fw, err := flate.NewWriter(w, c.compressionLevel) + if err != nil { + // Fallback to default compression if level is somehow invalid + fw, _ = flate.NewWriter(w, flate.DefaultCompression) + } return fw })router-tests/testenv/testenv.go (2)
2348-2384: Compression dial helper correctly mirrors existing retry logic; duplication is acceptable but extractable.The implementation is in line with
GraphQLWebsocketDialWithRetry: same retry/backoff behavior andErrBadHandshakehandling, with the only delta beingEnableCompression: true. That keeps behavior predictable and is enough to negotiatepermessage-deflate.If you want to avoid divergence long term, you could factor the common retry loop into a small internal helper taking a
*websocket.Dialer:Possible small refactor to share retry logic (optional)
func (e *Environment) GraphQLWebsocketDialWithRetry(header http.Header, query url.Values) (*websocket.Conn, *http.Response, error) { - dialer := websocket.Dialer{ - Subprotocols: []string{"graphql-transport-ws"}, - } - // ... - for i := 0; i <= maxSocketRetries; i++ { - // ... - conn, resp, err := dialer.Dial(urlStr, header) - // ... - } - return nil, nil, err + dialer := websocket.Dialer{ + Subprotocols: []string{"graphql-transport-ws"}, + } + return e.graphQLWebsocketDialWithRetryInternal(&dialer, header, query) } func (e *Environment) GraphQLWebsocketDialWithCompressionRetry(header http.Header, query url.Values) (*websocket.Conn, *http.Response, error) { - dialer := websocket.Dialer{ - Subprotocols: []string{"graphql-transport-ws"}, - EnableCompression: true, - } - // same loop as above... - return nil, nil, err + dialer := websocket.Dialer{ + Subprotocols: []string{"graphql-transport-ws"}, + EnableCompression: true, + } + return e.graphQLWebsocketDialWithRetryInternal(&dialer, header, query) } + +func (e *Environment) graphQLWebsocketDialWithRetryInternal( + dialer *websocket.Dialer, + header http.Header, + query url.Values, +) (*websocket.Conn, *http.Response, error) { + waitBetweenRetriesInMs := rand.Intn(10) + timeToSleep := time.Duration(waitBetweenRetriesInMs) * time.Millisecond + + var err error + for i := 0; i <= maxSocketRetries; i++ { + urlStr := e.GraphQLWebSocketSubscriptionURL() + if query != nil { + urlStr += "?" + query.Encode() + } + conn, resp, err := dialer.Dial(urlStr, header) + if resp != nil && err == nil { + return conn, resp, nil + } + if errors.Is(err, websocket.ErrBadHandshake) { + return conn, resp, err + } + if i != maxSocketRetries { + time.Sleep(timeToSleep) + timeToSleep *= 2 + } + } + return nil, nil, err +}
2403-2420: Compressed Init helper is consistent with the existing Init path and exposes the response for header assertions.This mirrors
InitGraphQLWebSocketConnectionclosely while (a) dialing through the compression-enabled helper and (b) returning the*http.Responseso tests can inspect negotiated extensions. Cleanup and ack handling are identical, so behavior stays aligned across both paths.If you want to DRY things up, you could have both Init helpers call a shared internal
initGraphQLWebSocketConnection(dial func(...) (*websocket.Conn, *http.Response, error)), but it’s non-essential.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
router-tests/testenv/testenv.go(2 hunks)router-tests/websocket_test.go(1 hunks)router/core/websocket.go(10 hunks)router/pkg/config/config.go(2 hunks)router/pkg/config/config.schema.json(1 hunks)router/pkg/config/fixtures/full.yaml(1 hunks)router/pkg/config/testdata/config_defaults.json(1 hunks)router/pkg/config/testdata/config_full.json(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-10-01T20:39:16.113Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2252
File: router-tests/telemetry/telemetry_test.go:9684-9693
Timestamp: 2025-10-01T20:39:16.113Z
Learning: Repo preference: In router-tests/telemetry/telemetry_test.go, keep strict > 0 assertions for request.operation.*Time (parsingTime, normalizationTime, validationTime, planningTime) in telemetry-related tests; do not relax to >= 0 unless CI flakiness is observed.
Applied to files:
router-tests/websocket_test.go
📚 Learning: 2025-08-28T09:18:10.121Z
Learnt from: endigma
Repo: wundergraph/cosmo PR: 2141
File: router-tests/http_subscriptions_test.go:100-108
Timestamp: 2025-08-28T09:18:10.121Z
Learning: In router-tests/http_subscriptions_test.go heartbeat tests, the message ordering should remain strict with data messages followed by heartbeat messages, as the timing is deterministic and known by design in the Cosmo router implementation.
Applied to files:
router-tests/websocket_test.go
📚 Learning: 2025-09-17T20:55:39.456Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2172
File: router/core/graph_server.go:0-0
Timestamp: 2025-09-17T20:55:39.456Z
Learning: The Initialize method in router/internal/retrytransport/manager.go has been updated to properly handle feature-flag-only subgraphs by collecting subgraphs from both routerConfig.GetSubgraphs() and routerConfig.FeatureFlagConfigs.ConfigByFeatureFlagName, ensuring all subgraphs receive retry configuration.
Applied to files:
router-tests/testenv/testenv.go
📚 Learning: 2025-11-19T15:13:57.821Z
Learnt from: dkorittki
Repo: wundergraph/cosmo PR: 2273
File: router/core/graphql_handler.go:0-0
Timestamp: 2025-11-19T15:13:57.821Z
Learning: In the Cosmo router (wundergraph/cosmo), error handling follows a two-phase pattern: (1) Prehandler phase handles request parsing, validation, and setup errors using `httpGraphqlError` and `writeOperationError` (in files like graphql_prehandler.go, operation_processor.go, parse_multipart.go, batch.go); (2) Execution phase handles resolver execution errors using `WriteError` in GraphQLHandler.ServeHTTP. Because all `httpGraphqlError` instances are caught in the prehandler before ServeHTTP is invoked, any error type checks for `httpGraphqlError` in the execution-phase WriteError method are unreachable code.
Applied to files:
router-tests/testenv/testenv.go
📚 Learning: 2025-07-21T15:06:36.664Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.
Applied to files:
router/pkg/config/config.schema.jsonrouter/pkg/config/testdata/config_full.json
📚 Learning: 2025-07-21T14:46:34.879Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
Applied to files:
router/pkg/config/testdata/config_full.json
🧬 Code graph analysis (1)
router/core/websocket.go (1)
router/pkg/config/config.go (1)
WebSocketConfiguration(710-727)
🔇 Additional comments (15)
router/pkg/config/fixtures/full.yaml (1)
407-409: LGTM!The compression configuration is correctly placed under the websocket section with sensible values. Level 6 provides a good balance between compression ratio and CPU usage.
router/pkg/config/testdata/config_defaults.json (1)
441-445: LGTM!Default configuration correctly has compression disabled by default, which is appropriate for backward compatibility. Level 6 is a sensible default.
router/pkg/config/testdata/config_full.json (1)
821-825: LGTM!The full configuration test data correctly reflects the compression settings from the YAML fixture.
router/core/websocket.go (6)
93-99: LGTM!The compression initialization correctly validates the compression level and falls back to
flate.DefaultCompressionfor out-of-range values. The check ensures user-provided values are within the valid 1-9 range for explicit compression levels.
169-182: LGTM!The connection wrapper correctly stores compression state for use in read/write operations.
194-239: LGTM!The compressed frame reading logic correctly:
- Unmasks client frames (per WebSocket spec, client-to-server frames are masked)
- Delegates control frame handling appropriately
- Checks the RSV1 bit to determine if decompression is needed
- Falls back to standard reading when compression is disabled
The read timeout set earlier in the function protects against infinite loops.
400-421: LGTM!The compression negotiation is well-implemented:
- Using
ServerNoContextTakeoverandClientNoContextTakeoveris a good choice for server-side implementations as it reduces memory overhead per connection- The negotiation only proceeds if the upgrade succeeds (
err == nil)- The
Accepted()check correctly determines if the client agreed to compression
256-261: LGTM!The compression path is correctly integrated with proper mutex handling.
355-358: LGTM!Handler correctly stores compression configuration for use during connection upgrade.
router-tests/websocket_test.go (3)
2395-2434: LGTM!This test correctly validates the compression negotiation and data transmission when both server and client support permessage-deflate. Good verification of the
Sec-WebSocket-Extensionsheader.
2436-2474: LGTM!This test correctly validates the fallback behavior when the server has compression disabled. It ensures the client can still communicate without compression when negotiation fails.
2476-2514: LGTM!Good test coverage for custom compression levels. The test uses a query with nested fields which helps validate compression/decompression of larger payloads.
router/pkg/config/config.schema.json (1)
563-581: WebSocket compression schema is well-scoped and consistent with existing config style.The new
websocket.compressionblock cleanly adds permessage-deflate tuning:enableddefaults tofalse,levelis constrained to 1–9 with a sensible default of 6, andadditionalProperties: falseprevents accidental typos. This aligns with the rest of the WebSocket config and should integrate smoothly with the Go config types.router/pkg/config/config.go (2)
725-727: LGTM!The new
Compressionfield is properly integrated into theWebSocketConfigurationstruct with appropriate YAML tags and documentation.
729-735: Compression level validation is properly enforced at runtime.The struct definition is correct and the 1-9 range validation is implemented in router/core/websocket.go (lines 96-98). Invalid values are silently replaced with flate.DefaultCompression, and the documented range in the comment is accurate.
|
@coderabbitai generate docstrings |
✅ Actions performedInitiated docstring generation; will generate only if new commits exist. |
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @dbinnersley. * #2424 (comment) The following files were modified: * `router/core/websocket.go`
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
dbinnersley
force-pushed
the
per-message-deflate
branch
from
4f0907c to
6ac365c
Compare
dbinnersley
requested review from
a team,
Aenimus,
JivusAyrus,
thisisnithin and
wilsonrivera
as code owners
There was a problem hiding this comment.
Actionable comments posted: 5
Fix all issues with AI Agents 🤖
In @controlplane/src/core/services/WebSessionAuthenticator.ts:
- Around line 48-66: The session userId comparison is done with toLowerCase()
but the subsequent user lookup uses the original decryptedJwt.iss, causing
inconsistency; normalize decryptedJwt.iss once (e.g., const normalizedIssuer =
decryptedJwt.iss?.toLowerCase()) and use normalizedIssuer for both the
existingSessions[0].userId comparison (compare
existingSessions[0].userId.toLowerCase() === normalizedIssuer) and for the user
lookup (userRepository.byId(normalizedIssuer)), or alternatively remove the
toLowerCase() here and ensure session/JWT creation always stores a normalized
userId—just make the same normalization used for equality also used when calling
userRepository.byId.
In @graphqlmetrics/CHANGELOG.md:
- Around line 7-12: Update the release date on the 0.37.0 changelog header from
"2025-12-15" to "2024-12-15" to match the PR timeline; locate the header line
beginning "# [0.37.0](... ) (2025-12-15)" in CHANGELOG.md and replace the year
accordingly, then scan nearby changelog entries to ensure date format
consistency with the studio CHANGELOG.
In @router-tests/modules/context_error_field_test.go:
- Around line 62-98: The test's GraphQL query doesn't request any
Products-originating fields so the Products subgraph isn't invoked; update the
GraphQLRequest in the test (inside the t.Run "error is captured in context when
subgraph fails") to include an Employee field that originates from Products
(e.g., request the products field on Employee) so the router will call the
Products subgraph and trigger the configured failure, leaving the rest of the
assertions about "errors" and the "X-Has-Error" header intact.
In @studio/CHANGELOG.md:
- Around line 7-19: The changelog entries for versions "0.148.0" and "0.147.0"
have incorrect release dates ("2025-12-15"); update those date strings to
"2024-12-15" in the corresponding headings so the timestamps match the PR
creation timeline for the entries referencing commits/PRs like 84c1ba4 and
a53bb35.
In @studio/package.json:
- Line 97: The Next.js dependency in package.json is pinned to "next": "15.4.10"
which contains critical security patches; before merging, update dependencies
and validate no regressions by: bumping/locking to 15.4.10 (or latest patched),
running npm/yarn install to refresh lockfile, executing the full test suite
(unit, integration, e2e), building the app (next build) and performing smoke
tests for Server Actions and SSR endpoints, running a security audit (npm audit
or Snyk) and reviewing Next.js changelogs for breaking changes introduced in
15.4.9/15.4.10, and committing the updated lockfile and any test fixes if issues
are found.
🧹 Nitpick comments (4)
controlplane/src/core/controllers/auth.ts (1)
1-1: Type safety improvements look good, but consider PR scope.The addition of explicit type aliases for request/response pairs is a solid refactor that enhances type safety and developer experience. The types are correctly structured with appropriate Querystring shapes for routes that need them.
However, this file refactors authentication controller types while the PR objective focuses on WebSocket per-message deflate compression. Consider whether bundling unrelated type safety improvements into a feature PR is appropriate for your project's workflow, or if these would be better suited for a separate housekeeping PR.
Also applies to: 42-73
controlplane/src/core/auth-utils.ts (1)
302-312: Potential redundancy in expiration checks.The two expiration checks may overlap when sessions are properly renewed:
- First check:
session.expiresAt <= now- Second check:
addSeconds(updatedAt ?? createdAt, DEFAULT_SESSION_MAX_AGE_SEC) <= nowSince
renewSessionsets bothexpiresAt = addSeconds(now, DEFAULT_SESSION_MAX_AGE_SEC)andupdatedAt = nowsimultaneously (lines 355, 363), these checks become equivalent for renewed sessions.The defense-in-depth approach is acceptable, but consider clarifying the intent with a comment explaining when each check would independently trigger (e.g., legacy sessions, database inconsistencies).
router/core/websocket.go (1)
285-302: Silently ignored error fromflate.NewWriter.Line 289 discards the error from
flate.NewWriter. While errors are rare for valid compression levels (1-9 orDefaultCompression), if an invalid level slips through, this would silently produce a nil writer, causing a panic on line 292.Given the validation at lines 96-98, this should be safe, but defensive error handling would be more robust.
Proposed fix
func (c *wsConnectionWrapper) writeCompressed(data []byte) error { var buf bytes.Buffer writer := wsflate.NewWriter(&buf, func(w io.Writer) wsflate.Compressor { - fw, _ := flate.NewWriter(w, c.compressionLevel) + fw, err := flate.NewWriter(w, c.compressionLevel) + if err != nil { + // Fallback to default compression if level is somehow invalid + fw, _ = flate.NewWriter(w, flate.DefaultCompression) + } return fw })router-tests/testenv/testenv.go (1)
2348-2384: Consider extracting shared retry logic to reduce duplication.
GraphQLWebsocketDialWithCompressionRetryis nearly identical toGraphQLWebsocketDialWithRetry(lines 2313-2346), differing only inEnableCompression: true. While separate methods provide clarity, the retry logic duplication could be consolidated.Suggested refactor
func (e *Environment) graphQLWebsocketDialWithRetryInternal(header http.Header, query url.Values, enableCompression bool) (*websocket.Conn, *http.Response, error) { dialer := websocket.Dialer{ Subprotocols: []string{"graphql-transport-ws"}, EnableCompression: enableCompression, } // ... rest of retry logic } func (e *Environment) GraphQLWebsocketDialWithRetry(header http.Header, query url.Values) (*websocket.Conn, *http.Response, error) { return e.graphQLWebsocketDialWithRetryInternal(header, query, false) } func (e *Environment) GraphQLWebsocketDialWithCompressionRetry(header http.Header, query url.Values) (*websocket.Conn, *http.Response, error) { return e.graphQLWebsocketDialWithRetryInternal(header, query, true) }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
keycloak/theme/package-lock.jsonis excluded by!**/package-lock.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yamlrouter-tests/go.sumis excluded by!**/*.sumrouter/go.sumis excluded by!**/*.sum
📒 Files selected for processing (44)
.github/workflows/protographic.yamlcli/CHANGELOG.mdcli/package.jsoncodecov.yamlcomposition/CHANGELOG.mdcomposition/package.jsoncontrolplane/CHANGELOG.mdcontrolplane/package.jsoncontrolplane/src/core/auth-utils.tscontrolplane/src/core/build-server.tscontrolplane/src/core/controllers/auth.tscontrolplane/src/core/repositories/analytics/MetricsRepository.tscontrolplane/src/core/repositories/analytics/SubgraphMetricsRepository.tscontrolplane/src/core/services/WebSessionAuthenticator.tsdemo/go.modgraphqlmetrics/CHANGELOG.mdgraphqlmetrics/package.jsonkeycloak/CHANGELOG.mdkeycloak/package.jsonpackage.jsonprotographic/CHANGELOG.mdprotographic/package.jsonrouter-tests/go.modrouter-tests/modules/context-error/module.gorouter-tests/modules/context_error_field_test.gorouter-tests/testenv/testenv.gorouter-tests/websocket_test.gorouter/CHANGELOG.mdrouter/cmd/plan_generator.gorouter/core/context.gorouter/core/graphql_handler.gorouter/core/supervisor_instance.gorouter/core/websocket.gorouter/go.modrouter/package.jsonrouter/pkg/config/config.gorouter/pkg/config/config.schema.jsonrouter/pkg/config/fixtures/full.yamlrouter/pkg/config/testdata/config_defaults.jsonrouter/pkg/config/testdata/config_full.jsonshared/CHANGELOG.mdshared/package.jsonstudio/CHANGELOG.mdstudio/package.json
✅ Files skipped from review due to trivial changes (7)
- controlplane/CHANGELOG.md
- shared/package.json
- keycloak/package.json
- composition/package.json
- controlplane/package.json
- protographic/package.json
- shared/CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (5)
- router/pkg/config/fixtures/full.yaml
- router/pkg/config/config.go
- router-tests/websocket_test.go
- router/pkg/config/testdata/config_full.json
- router/pkg/config/config.schema.json
🧰 Additional context used
🧠 Learnings (13)
📚 Learning: 2025-11-17T20:36:10.317Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2337
File: cli/src/commands/router/commands/compose.ts:0-0
Timestamp: 2025-11-17T20:36:10.317Z
Learning: In the `cli/src/commands/router/commands/compose.ts` file, the `router compose` command uses a static/constant UUID ('00000000-0000-0000-0000-000000000000') for the `schemaVersionId` because this command is for local router config generation and testing only. The version attribute is meant for the schema registry to use in production, so versioning is not important for local compositions.
Applied to files:
router/package.json
📚 Learning: 2025-08-29T10:28:04.846Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1749-1751
Timestamp: 2025-08-29T10:28:04.846Z
Learning: In the controlplane codebase, authentication and authorization checks (including organization scoping) are handled at the service layer in files like unlinkSubgraph.ts before calling repository methods. Repository methods like unlinkSubgraph() in SubgraphRepository.ts can focus purely on data operations without redundant security checks.
Applied to files:
controlplane/src/core/services/WebSessionAuthenticator.ts
📚 Learning: 2026-01-05T11:53:49.921Z
Learnt from: asoorm
Repo: wundergraph/cosmo PR: 2438
File: router-tests/go.mod:15-15
Timestamp: 2026-01-05T11:53:49.921Z
Learning: For mcp-go dependency in wundergraph/cosmo, patch releases may not have formal release notes published. Tag diffs should be reviewed to verify patch-level changes contain no API surface changes or breaking behavior before adopting new patch versions.
Applied to files:
keycloak/CHANGELOG.mdgraphqlmetrics/package.jsoncli/CHANGELOG.mdstudio/CHANGELOG.mdcomposition/CHANGELOG.mdgraphqlmetrics/CHANGELOG.mdprotographic/CHANGELOG.mdrouter/CHANGELOG.md
📚 Learning: 2025-08-15T10:21:45.838Z
Learnt from: StarpTech
Repo: wundergraph/cosmo PR: 2142
File: helm/cosmo/Chart.yaml:0-0
Timestamp: 2025-08-15T10:21:45.838Z
Learning: In the WunderGraph Cosmo project, helm chart version upgrades and README badge synchronization are handled in separate helm release PRs, not in the initial version bump PRs.
Applied to files:
keycloak/CHANGELOG.mdcli/CHANGELOG.mdstudio/CHANGELOG.mdcomposition/CHANGELOG.mdgraphqlmetrics/CHANGELOG.mdprotographic/CHANGELOG.mdrouter/CHANGELOG.md
📚 Learning: 2025-08-20T22:13:25.222Z
Learnt from: StarpTech
Repo: wundergraph/cosmo PR: 2157
File: router-tests/go.mod:16-16
Timestamp: 2025-08-20T22:13:25.222Z
Learning: github.com/mark3labs/mcp-go v0.38.0 has regressions and should not be used in the wundergraph/cosmo project. v0.36.0 is the stable version that should be used across router-tests and other modules.
Applied to files:
router-tests/go.modrouter/go.modgraphqlmetrics/CHANGELOG.mdrouter/CHANGELOG.md
📚 Learning: 2025-10-01T20:39:16.113Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2252
File: router-tests/telemetry/telemetry_test.go:9684-9693
Timestamp: 2025-10-01T20:39:16.113Z
Learning: Repo preference: In router-tests/telemetry/telemetry_test.go, keep strict > 0 assertions for request.operation.*Time (parsingTime, normalizationTime, validationTime, planningTime) in telemetry-related tests; do not relax to >= 0 unless CI flakiness is observed.
Applied to files:
router-tests/go.mod
📚 Learning: 2025-11-19T15:13:57.821Z
Learnt from: dkorittki
Repo: wundergraph/cosmo PR: 2273
File: router/core/graphql_handler.go:0-0
Timestamp: 2025-11-19T15:13:57.821Z
Learning: In the Cosmo router (wundergraph/cosmo), error handling follows a two-phase pattern: (1) Prehandler phase handles request parsing, validation, and setup errors using `httpGraphqlError` and `writeOperationError` (in files like graphql_prehandler.go, operation_processor.go, parse_multipart.go, batch.go); (2) Execution phase handles resolver execution errors using `WriteError` in GraphQLHandler.ServeHTTP. Because all `httpGraphqlError` instances are caught in the prehandler before ServeHTTP is invoked, any error type checks for `httpGraphqlError` in the execution-phase WriteError method are unreachable code.
Applied to files:
router-tests/modules/context-error/module.gorouter/core/graphql_handler.gorouter-tests/modules/context_error_field_test.go
📚 Learning: 2025-08-26T17:19:28.178Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2167
File: router/internal/expr/retry_context.go:3-10
Timestamp: 2025-08-26T17:19:28.178Z
Learning: In the Cosmo router codebase, prefer using netErr.Timeout() checks over explicit context.DeadlineExceeded checks for timeout detection in retry logic, as Go's networking stack typically wraps context deadline exceeded errors in proper net.Error implementations.
Applied to files:
router-tests/modules/context-error/module.go
📚 Learning: 2025-09-17T20:55:39.456Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2172
File: router/core/graph_server.go:0-0
Timestamp: 2025-09-17T20:55:39.456Z
Learning: The Initialize method in router/internal/retrytransport/manager.go has been updated to properly handle feature-flag-only subgraphs by collecting subgraphs from both routerConfig.GetSubgraphs() and routerConfig.FeatureFlagConfigs.ConfigByFeatureFlagName, ensuring all subgraphs receive retry configuration.
Applied to files:
router-tests/testenv/testenv.go
📚 Learning: 2025-09-24T12:54:00.765Z
Learnt from: endigma
Repo: wundergraph/cosmo PR: 2222
File: router-tests/websocket_test.go:2238-2302
Timestamp: 2025-09-24T12:54:00.765Z
Learning: The wundergraph/cosmo project uses Go 1.25 (Go 1.25 minimum), so fmt.Appendf and other newer Go standard library functions are available and can be used without compatibility concerns.
Applied to files:
demo/go.mod
📚 Learning: 2025-09-24T12:54:00.765Z
Learnt from: endigma
Repo: wundergraph/cosmo PR: 2222
File: router-tests/websocket_test.go:2238-2302
Timestamp: 2025-09-24T12:54:00.765Z
Learning: The wundergraph/cosmo project uses Go 1.25 (Go 1.23+ minimum), so fmt.Appendf and other newer Go standard library functions are available and can be used without compatibility concerns.
Applied to files:
demo/go.mod
📚 Learning: 2025-08-20T10:08:17.857Z
Learnt from: endigma
Repo: wundergraph/cosmo PR: 2155
File: router/core/router.go:1857-1866
Timestamp: 2025-08-20T10:08:17.857Z
Learning: router/pkg/config/config.schema.json forbids null values for traffic_shaping.subgraphs: additionalProperties references $defs.traffic_shaping_subgraph_request_rule with type "object". Therefore, in core.NewSubgraphTransportOptions, dereferencing each subgraph rule pointer is safe under schema-validated configs, and a nil-check is unnecessary.
Applied to files:
router/core/graphql_handler.go
📚 Learning: 2025-08-28T09:17:49.477Z
Learnt from: endigma
Repo: wundergraph/cosmo PR: 2141
File: router-tests/http_subscriptions_test.go:17-55
Timestamp: 2025-08-28T09:17:49.477Z
Learning: The Cosmo router uses a custom, intentionally rigid multipart implementation for GraphQL subscriptions. The multipart parsing in test files should remain strict and not be made more tolerant, as this rigidity is by design.
Applied to files:
router/CHANGELOG.md
🧬 Code graph analysis (8)
router/core/context.go (1)
router/pkg/pubsub/datasource/error.go (1)
Error(3-6)
router/core/websocket.go (1)
router/pkg/config/config.go (1)
WebSocketConfiguration(710-727)
router-tests/modules/context-error/module.go (2)
router/core/context.go (1)
RequestContext(61-145)router/core/modules.go (3)
Module(52-54)ModuleInfo(44-50)RouterOnRequestHandler(113-115)
controlplane/src/core/controllers/auth.ts (4)
controlplane/src/types/index.ts (2)
CustomAccessTokenClaims(456-462)UserSession(448-450)controlplane/src/core/crypto/jwt.ts (4)
decodeJWT(104-106)cosmoIdpHintCookieName(17-17)DEFAULT_SESSION_MAX_AGE_SEC(9-9)encrypt(60-69)controlplane/src/core/repositories/NamespaceRepository.ts (2)
NamespaceRepository(9-188)DefaultNamespace(7-7)controlplane/src/core/errors/errors.ts (1)
AuthenticationError(19-19)
router-tests/testenv/testenv.go (2)
router/internal/wsproto/proto.go (1)
Subprotocols(52-58)demo/cmd/benchmarksubscriptions/benchmark_test.go (1)
WebSocketMessage(167-171)
router/core/graphql_handler.go (1)
router/internal/expr/expr.go (1)
Context(35-39)
controlplane/src/core/auth-utils.ts (3)
controlplane/src/core/crypto/jwt.ts (1)
DEFAULT_SESSION_MAX_AGE_SEC(9-9)controlplane/src/core/errors/errors.ts (1)
AuthenticationError(19-19)controlplane/src/db/schema.ts (1)
sessions(1147-1169)
router-tests/modules/context_error_field_test.go (3)
router-tests/utils.go (1)
ConfigureAuth(45-57)router-tests/modules/context-error/module.go (1)
ContextErrorModule(11-13)router/core/router.go (3)
WithAccessController(1868-1872)WithModulesConfig(1708-1712)WithCustomModules(1815-1819)
🪛 markdownlint-cli2 (0.18.1)
studio/CHANGELOG.md
9-9: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
15-15: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
composition/CHANGELOG.md
9-9: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
graphqlmetrics/CHANGELOG.md
9-9: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
protographic/CHANGELOG.md
9-9: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
router/CHANGELOG.md
21-21: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🔇 Additional comments (40)
controlplane/src/core/controllers/auth.ts (1)
77-77: Handler typing is correctly implemented.All route handlers have been consistently updated to use the strongly-typed request/response parameters. The type constraints align with the actual query parameters accessed in each handler body, providing compile-time safety without any behavioral changes.
Also applies to: 126-126, 147-147, 427-427, 442-442
controlplane/src/core/auth-utils.ts (2)
343-347: Session expiration check placement looks good.The session expiration is correctly validated before attempting to refresh tokens with the IdP. This prevents unnecessary external calls for already-expired sessions.
351-385: Session renewal logic is well-structured.The changes correctly:
- Capture current time once (
now) and use it consistently- Update both
expiresAtandupdatedAtatomically in the same database update- Set JWT
maxAgeInSecondstoDEFAULT_SESSION_MAX_AGE_SECfor consistency- Pass the computed
expiresAttocreateSessionCookieThis ensures session state is synchronized across the database, JWT, and cookie.
controlplane/src/core/build-server.ts (1)
249-249: LGTM!The updated constructor call correctly passes
fastify.dbas the first argument, aligning with the newWebSessionAuthenticatorsignature that requires a database connection for server-side session validation.controlplane/src/core/services/WebSessionAuthenticator.ts (1)
19-24: Constructor signature update is properly integrated.The addition of the
dbparameter enables server-side session lookups. The parameter ordering (db, secret, repository) follows a common pattern where infrastructure dependencies precede configuration and domain-specific dependencies. Based on learnings, authentication checks at the service layer are the appropriate location for this validation.router/pkg/config/testdata/config_defaults.json (1)
442-444: LGTM! Conservative compression defaults.The WebSocket compression configuration defaults are appropriate:
- Disabled by default ensures backward compatibility
- Compression level 6 provides a good balance between speed and compression ratio
router/package.json (1)
3-3: Version bump looks correct for the feature addition.The version increment from 0.272.0 to 0.273.2 is appropriate for adding WebSocket compression support. The AI summary mentions that router/CHANGELOG.md documents version 0.273.2 with a Bug Fix entry, which aligns with this version bump.
keycloak/CHANGELOG.md (1)
7-12: LGTM! Well-formatted changelog entry.The changelog entry correctly documents the js-yaml vulnerability fix with proper version increment (0.11.0 → 0.11.1), follows conventional commits format, and includes all necessary references.
protographic/CHANGELOG.md (1)
7-12: LGTM! Standard changelog entry format.The changelog entry correctly documents the test coverage feature addition. The markdownlint warning about heading increment (H1 → H3) is a false positive—this is the standard format for conventional commit changelogs where version headers (H1) are followed directly by section headers (H3).
package.json (1)
71-71: Upgrade to Next.js 15.4.10 addresses critical security vulnerabilities.Version 15.4.10 exists and is the recommended patched release for the 15.4.x line. This upgrade fixes multiple critical security advisories:
- CVE-2025-55182: RCE in React Server Components (fixed in 15.4.8)
- CVE-2025-55184, CVE-2025-67779, CVE-2025-55183: DoS and source-code-exposure in Server Components (fixed in 15.4.10)
Next.js recommends upgrading affected App Router apps to 15.4.10 or later immediately.
composition/CHANGELOG.md (1)
7-12: LGTM!The changelog entry appropriately documents the version bump with a clear feature description that aligns with the test coverage additions in this PR.
cli/package.json (2)
3-3: Version bump looks correct.The version increment from 0.102.4 to 0.102.6 is appropriate for this release.
69-69: Update to js-yaml 4.1.1 is safe.The patch update is routine with no security advisories. This is the latest version, published by the maintainer and containing only the standard dependency (argparse).
graphqlmetrics/package.json (1)
3-3: LGTM!The minor version bump from 0.36.2 to 0.37.0 appropriately reflects the new WebSocket compression features being added in this PR.
router/go.mod (1)
67-67: Dependency update addresses critical security vulnerability.The update to expr-lang/expr v1.17.7 is a critical security patch that fixes CVE-2025-68156 (Denial of Service via unbounded recursion in builtin functions). The version exists and is stable; this update is recommended.
router/cmd/plan_generator.go (1)
107-107: FromCgroup provider is the correct choice and already in use.The code correctly uses
memlimit.FromCgroup, which auto-detects and handles both cgroup v1 and v2 environments. This is the standard, adaptive choice for containerized applications.FromCgroupHybridis specifically intended only for hybrid/mixed cgroup environments where both v1 and v2 interfaces coexist—not for standard Docker or Kubernetes deployments. No compatibility issues exist with this provider across cgroup v1, v2, or different container runtimes (Docker, Kubernetes, containerd, etc.).demo/go.mod (1)
57-57: expr-lang/expr patch bump looks reasonable; just confirm upstream changelogMoving
github.com/expr-lang/exprtov1.17.7is a normal patch update and should be safe, but since it’s an external dependency, please double-check its release notes for any behavioral changes that might affect existing expression usage.cli/CHANGELOG.md (1)
7-15: Changelog entries for 0.102.6 / 0.102.5 are consistent and clearThe new sections correctly document the js-yaml vulnerability fix and the version-bump-only release, matching the established changelog structure.
router/CHANGELOG.md (1)
7-23: Router changelog entries for 0.273.2 / 0.273.1 / 0.273.0 look goodThe new router release notes clearly describe the cgroup policy bugfix, error exposure for custom modules, and the added coverage, and they align with the existing changelog formatting.
router-tests/go.mod (1)
73-73: LGTM: Routine indirect dependency update.The patch-level update of
github.com/expr-lang/exprfrom v1.17.6 to v1.17.7 is a standard maintenance update for an indirect dependency.controlplane/src/core/repositories/analytics/SubgraphMetricsRepository.ts (2)
100-100: Verify the multiplier type change from UInt32 to Float64.The change from
{multiplier:UInt32}to{multiplier:Float64}affects the division precision in request rate calculations. This switches from integer division to floating-point division.Please confirm:
- Is this change intentional to improve metric precision?
- Have the metric calculations been tested to ensure accuracy?
- Are there any backward compatibility concerns with existing data or dashboards?
The same change appears at Line 134 in the top-5 aggregation query.
134-134: Verify consistency of Float64 type change.This line also changes the multiplier type from
UInt32toFloat64, consistent with the change at Line 100. Please ensure this change is tested across all affected queries.codecov.yaml (2)
17-38: LGTM: Improved path specificity for coverage flags.Adding trailing slashes to the flag paths (e.g.,
router/instead ofrouter) makes the path matching more explicit and precise. This ensures coverage is calculated only for files within those specific directories, avoiding potential matches with similarly-named directories.
40-46: LGTM: Explicit comment configuration added.The new comment section adds explicit configuration for how Codecov displays coverage reports in PR comments. The settings are reasonable:
- Layout shows diff, flags, and files
- Carryforward flags are displayed for transparency
- Changes are not required for comments
.github/workflows/protographic.yaml (1)
44-52: LGTM! Enhanced Codecov configuration.The additional inputs (
coverage-path,retention-days,codecov-token) improve test result reporting and retention management. The implementation is correct.However, note that this workflow change for the
protographicpackage appears unrelated to the PR's stated objective of adding WebSocket per-message deflate compression to the router.router/core/context.go (1)
143-144: LGTM! Clean error accessor implementation.The new
Error() errormethod provides public access to the request's error state, following the same pattern as other getters in theRequestContextinterface (Request(),Logger(), etc.). The implementation correctly returns the internal error field.Note: This change appears to be a general API enhancement rather than being directly related to the PR's WebSocket per-message deflate compression feature.
Also applies to: 479-482
studio/package.json (2)
3-3: Version bump skips 0.147.0.The version jumps from 0.146.0 to 0.148.0, skipping 0.147.0. This might indicate multiple releases were merged or a version was reserved for another purpose.
Please confirm this version skip is intentional.
138-138: LGTM! Correct dependency classification.Moving
@vitest/coverage-v8fromdependenciestodevDependenciesis the correct classification, as code coverage tools are only needed during development and testing, not at runtime.controlplane/src/core/repositories/analytics/MetricsRepository.ts (1)
88-88: The Float64 type is required for correct query behavior and should not be reverted to UInt32.While the multiplier is computed as an integer (
rangeInHours * 60), the divisor type in ClickHouse SQL controls whether division is performed as integer or floating-point arithmetic. Using UInt32 would cause truncation or precision loss in the division results. This change (PR #2411) was a deliberate bug fix to ensure accurate metrics calculations across MetricsRepository, SubgraphMetricsRepository, and AnalyticsDashboardViewRepository—not an unnecessary type conversion.Likely an incorrect or invalid review comment.
router/core/supervisor_instance.go (1)
44-44: The memory limit providermemlimit.FromCgroupat line 44 is the standard auto-detect provider for containerized environments. This choice is appropriate for typical Docker/Kubernetes deployments where cgroup versions are consistent (either v1 or v2, not mixed). If your deployments require support for hybrid cgroup environments (mixed v1/v2), verify that the target environments don't have this constraint, or consider explicitly usingmemlimit.FromCgroupHybridinstead.Likely an incorrect or invalid review comment.
router/core/websocket.go (3)
194-239: LGTM on the compressed read path.The frame-by-frame reading with proper masking, control frame handling, and RSV1-based decompression detection is correctly implemented. The loop properly handles interleaved control frames before data frames.
400-421: Compression negotiation properly respects client capabilities.The implementation correctly:
- Only offers compression when enabled in config
- Uses
NoContextTakeoverfor both sides (memory-safe, avoids state accumulation)- Checks
ext.Accepted()after upgrade to determine actual negotiated state- Passes
compressionNegotiated(not justcompressionEnabled) to the connectionThis ensures compression is only used when both server and client agree.
93-99: Compression level validation is reasonable.The check restricts levels to 1-9 (corresponding to
flate.BestSpeedthroughflate.BestCompression), defaulting invalid values toflate.DefaultCompression. This is a sensible user-facing API that avoids confusion with special values like 0 (no compression) or -2 (Huffman only).Consider documenting the valid range (1-9) in the configuration schema or config struct comments to guide users.
router/core/graphql_handler.go (1)
188-191: LGTM - Improved error tracking for subgraph errors.Adding
trackFinalResponseErrorwhenSubgraphErrors()is non-nil ensures these errors are properly captured in telemetry, providing better observability for partial failures in federated responses.router-tests/modules/context_error_field_test.go (2)
32-43: Module registered twice - verify this is intentional.The
ContextErrorModuleis added both tocfg.Modules(line 34) and viaWithCustomModules(line 42). Depending on how the router processes these, this could lead to the module being invoked twice or one registration overriding the other.If this is intentional for testing module loading from config vs programmatic registration, consider adding a comment. Otherwise, one registration method should suffice.
17-125: LGTM - Comprehensive test coverage for context error propagation.The tests properly cover three key scenarios:
- Authentication failure → 401 with error captured
- Subgraph failure → GraphQL errors with error captured
- Success → No error header
Using
t.Parallel()for all subtests is good for test performance.router-tests/testenv/testenv.go (1)
2403-2420: LGTM - Compression-enabled WebSocket initialization helper.The method correctly:
- Returns both the connection and response (needed to inspect negotiated headers)
- Follows the same initialization pattern as the non-compression variant
- Registers cleanup via
t.Cleanuprouter-tests/modules/context-error/module.go (3)
43-50: Write callscheckAndSetErroronly on first write - error set later won't be captured.Once
headerWrittenis true (after firstWriteorWriteHeader), subsequent writes don't recheckctx.Error(). If an error is set mid-response (e.g., during streaming), it won't be reflected in the header.This may be intentional since HTTP headers can't be modified after the first byte is sent. The current behavior captures errors known at response start.
74-84: Module registration looks correct.The
Module()method returns properModuleInfowith:
- Unique ID
- Low priority (1) for early execution
- Factory function that creates fresh instances
The interface guard at lines 87-89 ensures compile-time verification of interface compliance.
11-13: ErrorValue field causes race condition with concurrent requests.Modules are initialized once at startup and reused across all requests. The
ErrorValuefield at line 12 is shared across concurrent requests, causing data races when multiple requests write to it simultaneously at line 27. Store error values in request-scoped context instead.⛔ Skipped due to learnings
Learnt from: endigma Repo: wundergraph/cosmo PR: 2334 File: router/cmd/flightrecorder/module/module.go:83-97 Timestamp: 2025-11-24T13:44:40.478Z Learning: In the FlightRecorder module (router/cmd/flightrecorder/module/module.go), concurrent WriteTo() errors are acceptable and intentional. The module doesn't need multiple traces for events happening close together, so no synchronization is needed for concurrent writes.
| # [0.37.0](https://github.com/wundergraph/cosmo/compare/[email protected]@0.37.0) (2025-12-15) | ||
|
|
||
| ### Features | ||
|
|
||
| * add coverage for other services with tests ([#2405](https://github.com/wundergraph/cosmo/issues/2405)) ([a53bb35](https://github.com/wundergraph/cosmo/commit/a53bb351336dc400b8b28b235a28725704a83919)) (@SkArchon) | ||
|
|
There was a problem hiding this comment.
Verify release date in changelog entry.
Similar to the studio CHANGELOG, the release date shows "2025-12-15" but should likely be "2024-12-15" based on the PR creation date of 2024-12-19.
🔎 Proposed fix for the date
-# [0.37.0](https://github.com/wundergraph/cosmo/compare/[email protected]@0.37.0) (2025-12-15)
+# [0.37.0](https://github.com/wundergraph/cosmo/compare/[email protected]@0.37.0) (2024-12-15)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # [0.37.0](https://github.com/wundergraph/cosmo/compare/[email protected]@0.37.0) (2025-12-15) | |
| ### Features | |
| * add coverage for other services with tests ([#2405](https://github.com/wundergraph/cosmo/issues/2405)) ([a53bb35](https://github.com/wundergraph/cosmo/commit/a53bb351336dc400b8b28b235a28725704a83919)) (@SkArchon) | |
| # [0.37.0](https://github.com/wundergraph/cosmo/compare/[email protected]@0.37.0) (2024-12-15) | |
| ### Features | |
| * add coverage for other services with tests ([#2405](https://github.com/wundergraph/cosmo/issues/2405)) ([a53bb35](https://github.com/wundergraph/cosmo/commit/a53bb351336dc400b8b28b235a28725704a83919)) (@SkArchon) |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
9-9: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🤖 Prompt for AI Agents
In @graphqlmetrics/CHANGELOG.md around lines 7-12, Update the release date on
the 0.37.0 changelog header from "2025-12-15" to "2024-12-15" to match the PR
timeline; locate the header line beginning "# [0.37.0](... ) (2025-12-15)" in
CHANGELOG.md and replace the year accordingly, then scan nearby changelog
entries to ensure date format consistency with the studio CHANGELOG.
| t.Run("error is captured in context when subgraph fails", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| cfg := config.Config{ | ||
| Modules: map[string]interface{}{ | ||
| "contextErrorModule": contexterror.ContextErrorModule{}, | ||
| }, | ||
| } | ||
|
|
||
| testenv.Run(t, &testenv.Config{ | ||
| RouterOptions: []core.Option{ | ||
| core.WithModulesConfig(cfg.Modules), | ||
| core.WithCustomModules(&contexterror.ContextErrorModule{}), | ||
| }, | ||
| Subgraphs: testenv.SubgraphsConfig{ | ||
| Products: testenv.SubgraphConfig{ | ||
| Middleware: func(handler http.Handler) http.Handler { | ||
| return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| w.WriteHeader(http.StatusInternalServerError) | ||
| _, _ = w.Write([]byte(`{"errors":[{"message":"Internal server error","extensions":{"code":"INTERNAL_SERVER_ERROR"}}]}`)) | ||
| }) | ||
| }, | ||
| }, | ||
| }, | ||
| }, func(t *testing.T, xEnv *testenv.Environment) { | ||
| res := xEnv.MakeGraphQLRequestOK(testenv.GraphQLRequest{ | ||
| Query: `{ employees { id details { forename surname } notes } }`, | ||
| }) | ||
|
|
||
| // Verify the response contains errors from the subgraph failure | ||
| require.Contains(t, res.Body, "errors") | ||
| require.Contains(t, res.Body, "Failed to fetch from Subgraph") | ||
|
|
||
| // Verify the X-Has-Error header is set when subgraph fails | ||
| require.Equal(t, "true", res.Response.Header.Get("X-Has-Error")) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find GraphQL schema files in the repository
echo "=== Finding GraphQL schema files ==="
find . -type f -name "*.graphql" | head -20Repository: wundergraph/cosmo
Length of output: 1260
🏁 Script executed:
#!/bin/bash
# Also check if there are any hardcoded schema definitions in Go files
echo "=== Checking for schema definitions ==="
fd -e go -x grep -l "type Employee\|type.*Schema\|sdl\|schema" {} \; | grep -i "schema\|config\|testenv" | head -10Repository: wundergraph/cosmo
Length of output: 511
🏁 Script executed:
#!/bin/bash
# Look for test setup and subgraph configuration in testenv
echo "=== Searching for testenv configuration files ==="
find . -type f -name "*testenv*" -o -name "*subgraph*" | head -20Repository: wundergraph/cosmo
Length of output: 1202
🏁 Script executed:
#!/bin/bash
# Check the employees schema to understand the Employee type structure
echo "=== employees.graphql ==="
cat ./composition/tests/v1/test-data/employees.graphqlRepository: wundergraph/cosmo
Length of output: 1933
🏁 Script executed:
#!/bin/bash
# Check the products schema to see if it defines the notes field
echo "=== products.graphql ==="
cat ./composition/tests/v1/test-data/products.graphqlRepository: wundergraph/cosmo
Length of output: 1585
🏁 Script executed:
#!/bin/bash
# Check the family schema to understand the details field
echo "=== family.graphql ==="
cat ./composition/tests/v1/test-data/family.graphqlRepository: wundergraph/cosmo
Length of output: 1602
🏁 Script executed:
#!/bin/bash
# Let me check the full test file to understand the test setup better
cat router-tests/modules/context_error_field_test.goRepository: wundergraph/cosmo
Length of output: 4088
🏁 Script executed:
#!/bin/bash
# Check if there's a testenv setup file that might configure how subgraphs are routed
find . -type f -name "*.go" -path "*/testenv/*" | head -10Repository: wundergraph/cosmo
Length of output: 236
🏁 Script executed:
#!/bin/bash
# Check the testenv configuration to understand how subgraphs are initialized
echo "=== Examining testenv.go ==="
head -100 ./router-tests/testenv/testenv.goRepository: wundergraph/cosmo
Length of output: 2898
🏁 Script executed:
#!/bin/bash
# Look for how composed schema is built in testenv
rg "SubgraphsConfig|Subgraphs" ./router-tests/testenv/ -A 5 | head -50Repository: wundergraph/cosmo
Length of output: 3712
🏁 Script executed:
#!/bin/bash
# Check if there's a supergraph or composed schema definition
find ./router-tests -type f \( -name "*.graphql" -o -name "*schema*" \) | grep -v node_modulesRepository: wundergraph/cosmo
Length of output: 1908
🏁 Script executed:
#!/bin/bash
# Look for supergraph or composed schema in testenv testdata
echo "=== Checking testenv testdata for supergraph ==="
find ./router-tests/testenv/testdata -type f | head -20Repository: wundergraph/cosmo
Length of output: 1577
🏁 Script executed:
#!/bin/bash
# Check the config.json template to see what schema it uses
cat ./router-tests/testenv/testdata/config.json | head -100Repository: wundergraph/cosmo
Length of output: 2420
🏁 Script executed:
#!/bin/bash
# Search for how the supergraph/federated schema is built in testenv
rg "supergraph|composed|federation" ./router-tests/testenv/testenv.go -i | head -30Repository: wundergraph/cosmo
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Let's find where the supergraph or composed schema string is defined
rg "schema.*{" ./router-tests/testenv/ -A 10 | grep -A 10 "type Query\|type Employee\|notes" | head -80Repository: wundergraph/cosmo
Length of output: 142
🏁 Script executed:
#!/bin/bash
# Check if there's a hardcoded supergraph string in testenv
rg "notes.*String|Employee.*@key" ./router-tests/testenv/ | head -20Repository: wundergraph/cosmo
Length of output: 50374
🏁 Script executed:
#!/bin/bash
# Look for where subgraph configs reference their schemas
cat ./router-tests/testenv/testdata/config.json | jq '.engineConfig.datasourceConfigurations[] | select(.name == "employees" or .name == "products" or .name == "family") | {name, rootNodes}' 2>/dev/null | head -100Repository: wundergraph/cosmo
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Check if there's logic in testenv that explains when subgraphs are called
grep -r "products" ./router-tests/modules/context_error_field_test.go -B 5 -A 5Repository: wundergraph/cosmo
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Let's also check if there are other similar tests that use Products subgraph correctly
grep -r "Products.*Middleware\|products.*Middleware" ./router-tests --include="*.go" | head -5Repository: wundergraph/cosmo
Length of output: 420
🏁 Script executed:
#!/bin/bash
# Check what fields the Products subgraph actually provides in the composed schema
cat ./router-tests/testenv/testdata/config.json | jq '.engineConfig.datasourceConfigurations[] | select(.name == "products") | .rootNodes' 2>/dev/null | head -60Repository: wundergraph/cosmo
Length of output: 43
The test query doesn't request any fields from the Products subgraph, so failing it won't trigger the error being tested.
The query { employees { id details { forename surname } notes } } only requests fields that originate from the Employees and Family subgraphs:
idandnotescome from Employeesdetails { forename surname }come from Employees/Family
The Products subgraph only has an @override directive on the notes field, meaning it doesn't originate it—Employees does. In federated graph execution, the router only calls a subgraph when it needs fields that originate from that subgraph. Since this query doesn't request any Products-originated fields, the Products subgraph won't be contacted even though its middleware is configured to fail.
To test subgraph failure handling, the query should request a field that actually originates from Products, such as the products: [ProductName!]! field on Employee.
🤖 Prompt for AI Agents
In @router-tests/modules/context_error_field_test.go around lines 62-98, The
test's GraphQL query doesn't request any Products-originating fields so the
Products subgraph isn't invoked; update the GraphQLRequest in the test (inside
the t.Run "error is captured in context when subgraph fails") to include an
Employee field that originates from Products (e.g., request the products field
on Employee) so the router will call the Products subgraph and trigger the
configured failure, leaving the rest of the assertions about "errors" and the
"X-Has-Error" header intact.
| # [0.148.0](https://github.com/wundergraph/cosmo/compare/[email protected]@0.148.0) (2025-12-15) | ||
|
|
||
| ### Features | ||
|
|
||
| * upgrade NextJS ([#2410](https://github.com/wundergraph/cosmo/issues/2410)) ([84c1ba4](https://github.com/wundergraph/cosmo/commit/84c1ba4694e8a90e04278ee58d5520282567d8ae)) (@wilsonrivera) | ||
|
|
||
| # [0.147.0](https://github.com/wundergraph/cosmo/compare/[email protected]@0.147.0) (2025-12-15) | ||
|
|
||
| ### Features | ||
|
|
||
| * add coverage for other services with tests ([#2405](https://github.com/wundergraph/cosmo/issues/2405)) ([a53bb35](https://github.com/wundergraph/cosmo/commit/a53bb351336dc400b8b28b235a28725704a83919)) (@SkArchon) | ||
| * improve Studio navigation and namespace/organization switching ([#2408](https://github.com/wundergraph/cosmo/issues/2408)) ([923cd4f](https://github.com/wundergraph/cosmo/commit/923cd4f3efcc63004d884938a6ec6d5771fa38e0)) (@wilsonrivera) | ||
|
|
There was a problem hiding this comment.
Verify release dates in changelog entries.
The release dates show "2025-12-15" for both versions 0.148.0 and 0.147.0, but the PR was created on 2024-12-19. These dates appear to be typos and should likely be "2024-12-15" instead.
🔎 Proposed fix for the dates
-# [0.148.0](https://github.com/wundergraph/cosmo/compare/[email protected]@0.148.0) (2025-12-15)
+# [0.148.0](https://github.com/wundergraph/cosmo/compare/[email protected]@0.148.0) (2024-12-15)
### Features
* upgrade NextJS ([#2410](https://github.com/wundergraph/cosmo/issues/2410)) ([84c1ba4](https://github.com/wundergraph/cosmo/commit/84c1ba4694e8a90e04278ee58d5520282567d8ae)) (@wilsonrivera)
-# [0.147.0](https://github.com/wundergraph/cosmo/compare/[email protected]@0.147.0) (2025-12-15)
+# [0.147.0](https://github.com/wundergraph/cosmo/compare/[email protected]@0.147.0) (2024-12-15)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # [0.148.0](https://github.com/wundergraph/cosmo/compare/[email protected]@0.148.0) (2025-12-15) | |
| ### Features | |
| * upgrade NextJS ([#2410](https://github.com/wundergraph/cosmo/issues/2410)) ([84c1ba4](https://github.com/wundergraph/cosmo/commit/84c1ba4694e8a90e04278ee58d5520282567d8ae)) (@wilsonrivera) | |
| # [0.147.0](https://github.com/wundergraph/cosmo/compare/[email protected]@0.147.0) (2025-12-15) | |
| ### Features | |
| * add coverage for other services with tests ([#2405](https://github.com/wundergraph/cosmo/issues/2405)) ([a53bb35](https://github.com/wundergraph/cosmo/commit/a53bb351336dc400b8b28b235a28725704a83919)) (@SkArchon) | |
| * improve Studio navigation and namespace/organization switching ([#2408](https://github.com/wundergraph/cosmo/issues/2408)) ([923cd4f](https://github.com/wundergraph/cosmo/commit/923cd4f3efcc63004d884938a6ec6d5771fa38e0)) (@wilsonrivera) | |
| # [0.148.0](https://github.com/wundergraph/cosmo/compare/[email protected]@0.148.0) (2024-12-15) | |
| ### Features | |
| * upgrade NextJS ([#2410](https://github.com/wundergraph/cosmo/issues/2410)) ([84c1ba4](https://github.com/wundergraph/cosmo/commit/84c1ba4694e8a90e04278ee58d5520282567d8ae)) (@wilsonrivera) | |
| # [0.147.0](https://github.com/wundergraph/cosmo/compare/[email protected]@0.147.0) (2024-12-15) | |
| ### Features | |
| * add coverage for other services with tests ([#2405](https://github.com/wundergraph/cosmo/issues/2405)) ([a53bb35](https://github.com/wundergraph/cosmo/commit/a53bb351336dc400b8b28b235a28725704a83919)) (@SkArchon) | |
| * improve Studio navigation and namespace/organization switching ([#2408](https://github.com/wundergraph/cosmo/issues/2408)) ([923cd4f](https://github.com/wundergraph/cosmo/commit/923cd4f3efcc63004d884938a6ec6d5771fa38e0)) (@wilsonrivera) |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
9-9: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
15-15: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🤖 Prompt for AI Agents
In @studio/CHANGELOG.md around lines 7-19, The changelog entries for versions
"0.148.0" and "0.147.0" have incorrect release dates ("2025-12-15"); update
those date strings to "2024-12-15" in the corresponding headings so the
timestamps match the PR creation timeline for the entries referencing
commits/PRs like 84c1ba4 and a53bb35.
| "monaco-editor": "^0.45.0", | ||
| "monaco-graphql": "^1.4.0", | ||
| "next": "15.4.8", | ||
| "next": "15.4.10", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Next.js 15.4.10 release notes changelog breaking changes
💡 Result:
Key breaking changes in Next.js 15.4.10 (Next.js 15 series) — summary:
-
Async Request APIs: request-scoped APIs are now async. Affected APIs: cookies, headers, draftMode, params (in layout.js, page.js, route.js, default.js, generateMetadata, generateViewport) and searchParams (in page.js). Synchronous access shows warnings and a codemod is provided. [1][2]
-
Caching semantics: fetch, GET Route Handlers, and client navigations are not cached by default (no-store behavior); you must opt into caching. [1][2]
-
React / tooling alignment: Next.js 15 targets React 19 (minimum react/react-dom versions updated) and includes codemod/tooling to help upgrades. [1][2]
-
Miscellaneous breaking items called out in the release notes: updates to next/image (image pipeline changes e.g., sharp/squoosh swap), middleware/API import restrictions, defaults for SWC/minify and Node.js minimum version changes — see upgrade guide for full list. [1][3]
References:
- Next.js blog — Next.js 15 release notes. [Next.js 15]. [2]
- Next.js docs — Upgrading: Version 15 (migration guide). [3]
- Community roundup / changelog summaries (examples discussing image, middleware, Node.js, ESLint changes). [4]
If you want, I can run the codemod commands and list the exact file-level code changes you’ll need for cookies/headers/draftMode in your repo.
🌐 Web query:
Next.js 15.4.9 release notes
💡 Result:
Summary of Next.js 15.4.9 release notes
- Purpose: security patch for React Server Components vulnerabilities (Denial‑of‑Service and Server Actions source‑code exposure). [1][2]
- Key fixes: addresses CVE-2025-55184 (DoS) and CVE-2025-55183 (Server Actions/source‑code exposure) for Next.js 15.4.x by shipping the patched 15.4.9 release. Users on 15.4.x should upgrade to 15.4.9 immediately. [2][3]
- Upgrade guidance: update your project (e.g., npm install [email protected] or pnpm add [email protected]) and also upgrade React to the patched React versions recommended in the Vercel bulletin. [2]
Sources
- Next.js 15.4 blog post (release highlights). [1]
- Vercel security bulletin / upgrade instructions for CVE-2025-55184 and CVE-2025-55183. [2]
- GitHub security advisories listing 15.4.9 as a patched release. [3]
If you want, I can paste the exact Vercel upgrade commands and the React versions to update to.
Update includes critical security patches — verify testing before merging.
Next.js 15.4.10 includes critical security fixes (CVE-2025-55184 DoS and CVE-2025-55183 Server Actions source-code exposure) patched in 15.4.9. Ensure comprehensive testing and confirm no breaking changes introduced in these versions before deployment.
🤖 Prompt for AI Agents
In @studio/package.json around line 97, The Next.js dependency in package.json
is pinned to "next": "15.4.10" which contains critical security patches; before
merging, update dependencies and validate no regressions by: bumping/locking to
15.4.10 (or latest patched), running npm/yarn install to refresh lockfile,
executing the full test suite (unit, integration, e2e), building the app (next
build) and performing smoke tests for Server Actions and SSR endpoints, running
a security audit (npm audit or Snyk) and reviewing Next.js changelogs for
breaking changes introduced in 15.4.9/15.4.10, and committing the updated
lockfile and any test fixes if issues are found.
6ac365c to
6a5b04c
Compare
dbinnersley
force-pushed
the
per-message-deflate
branch
from
6a5b04c to
d36638c
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
router/core/websocket.go (1)
185-243: Compressed ReadJSON path does not handle fragmented messagesThe new
ReadJSONbranch forcompressionEnabledreads raw frames viaws.ReadFrameand treats a singleOpText/OpBinaryframe (optionally decompressed viawsflate.DecompressFrame) as the whole message. This has a couple of implications:
- Valid fragmented messages (
Text/Binaryfollowed by one or moreContinuationframes) are not assembled: continuation frames are effectively ignored, so large or fragmented messages would either fail to decompress/parse or be silently truncated.- For compressed messages, permessage‑deflate expects the compressed stream to cover the full message; decompressing only the first fragment will typically error out.
The previous uncompressed path delegated to
wsutil.ReadClientText, which handles fragmentation and masking for you. To avoid protocol‑level edge‑case bugs (especially with larger or fragmented frames), it would be safer to use the wsutil/wsflate primitives that operate at message level rather than per‑frame, or to explicitly accumulate continuation frames before decompressing.Functionally this is likely fine with current clients that send small, unfragmented frames, but it’s a real correctness risk if a client or intermediary starts fragmenting compressed messages.
Possible direction for a safer implementation (sketch, not drop‑in)
func (c *wsConnectionWrapper) ReadJSON(v any) error { @@ - if c.compressionEnabled { - // Read frames directly and handle compression - controlHandler := wsutil.ControlFrameHandler(c.conn, ws.StateServerSide) - for { - frame, err := ws.ReadFrame(c.conn) - ... - } - } else { - text, err = wsutil.ReadClientText(c.conn) - ... - } + if c.compressionEnabled { + // Consider using wsutil.Reader with wsflate integration here so that + // it assembles fragmented messages and handles control frames, then + // decompresses the full message payload before unmarshalling. + // + // e.g. pseudo‑code: + // rd := wsutil.NewReader(c.conn, ws.StateServerSide) + // h, err := rd.NextFrame() + // handle control frames via wsutil.ControlFrameHandler + // accumulate fragments while h.OpCode == Text/Binary/Continuation + // if compressed (RSV1), pass assembled payload through wsflate + // text = assembled (and possibly decompressed) payload + } else { + text, err = wsutil.ReadClientText(c.conn) + if err != nil { + return err + } + }Also applies to: 256-283, 285-303
🧹 Nitpick comments (2)
router-tests/testenv/testenv.go (1)
2348-2384: Compression‑aware dial helper looks correct; consider minor de‑duplicationLogic matches
GraphQLWebsocketDialWithRetryand correctly enablesgorilla/websocketpermessage‑deflate negotiation viaEnableCompression: true, including retry andErrBadHandshakehandling. To keep things DRY, you could factor both helpers through a shared internalgraphqlWebsocketDialWithRetry(enableCompression bool, ...)helper, but that’s optional given the small size.router-tests/websocket_test.go (1)
2395-2514: Compression tests cover key negotiation and behavior paths wellThe three new subtests nicely validate:
- Successful permessage‑deflate negotiation when both sides enable it (including end‑to‑end subscription under compression).
- No negotiation when the server disables compression, even if the client requests it.
- Behavior with a non‑default compression level.
If you want to extend coverage later, a small additional case where the server has compression enabled but the client does not request the extension would fully exercise the “server supports but client opts out” branch, but the current tests are already solid.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
router-tests/testenv/testenv.gorouter-tests/websocket_test.gorouter/core/websocket.gorouter/pkg/config/config.gorouter/pkg/config/config.schema.jsonrouter/pkg/config/fixtures/full.yamlrouter/pkg/config/testdata/config_defaults.jsonrouter/pkg/config/testdata/config_full.json
🚧 Files skipped from review as they are similar to previous changes (5)
- router/pkg/config/fixtures/full.yaml
- router/pkg/config/config.go
- router/pkg/config/testdata/config_full.json
- router/pkg/config/config.schema.json
- router/pkg/config/testdata/config_defaults.json
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-10-01T20:39:16.113Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2252
File: router-tests/telemetry/telemetry_test.go:9684-9693
Timestamp: 2025-10-01T20:39:16.113Z
Learning: Repo preference: In router-tests/telemetry/telemetry_test.go, keep strict > 0 assertions for request.operation.*Time (parsingTime, normalizationTime, validationTime, planningTime) in telemetry-related tests; do not relax to >= 0 unless CI flakiness is observed.
Applied to files:
router-tests/websocket_test.go
📚 Learning: 2025-08-28T09:18:10.121Z
Learnt from: endigma
Repo: wundergraph/cosmo PR: 2141
File: router-tests/http_subscriptions_test.go:100-108
Timestamp: 2025-08-28T09:18:10.121Z
Learning: In router-tests/http_subscriptions_test.go heartbeat tests, the message ordering should remain strict with data messages followed by heartbeat messages, as the timing is deterministic and known by design in the Cosmo router implementation.
Applied to files:
router-tests/websocket_test.go
📚 Learning: 2025-09-17T20:55:39.456Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2172
File: router/core/graph_server.go:0-0
Timestamp: 2025-09-17T20:55:39.456Z
Learning: The Initialize method in router/internal/retrytransport/manager.go has been updated to properly handle feature-flag-only subgraphs by collecting subgraphs from both routerConfig.GetSubgraphs() and routerConfig.FeatureFlagConfigs.ConfigByFeatureFlagName, ensuring all subgraphs receive retry configuration.
Applied to files:
router-tests/testenv/testenv.go
📚 Learning: 2025-11-19T15:13:57.821Z
Learnt from: dkorittki
Repo: wundergraph/cosmo PR: 2273
File: router/core/graphql_handler.go:0-0
Timestamp: 2025-11-19T15:13:57.821Z
Learning: In the Cosmo router (wundergraph/cosmo), error handling follows a two-phase pattern: (1) Prehandler phase handles request parsing, validation, and setup errors using `httpGraphqlError` and `writeOperationError` (in files like graphql_prehandler.go, operation_processor.go, parse_multipart.go, batch.go); (2) Execution phase handles resolver execution errors using `WriteError` in GraphQLHandler.ServeHTTP. Because all `httpGraphqlError` instances are caught in the prehandler before ServeHTTP is invoked, any error type checks for `httpGraphqlError` in the execution-phase WriteError method are unreachable code.
Applied to files:
router-tests/testenv/testenv.go
🧬 Code graph analysis (3)
router-tests/websocket_test.go (2)
router/pkg/config/config.go (2)
Config(1041-1116)WebSocketConfiguration(710-727)demo/cmd/benchmarksubscriptions/benchmark_test.go (1)
WebSocketMessage(167-171)
router/core/websocket.go (1)
router/pkg/config/config.go (1)
WebSocketConfiguration(710-727)
router-tests/testenv/testenv.go (1)
router/internal/wsproto/proto.go (1)
Subprotocols(52-58)
🔇 Additional comments (2)
router-tests/testenv/testenv.go (1)
2403-2420: Init helper returning*http.Responseis consistent and safe
InitGraphQLWebSocketConnectionWithCompressionmirrors the existing init helper, adds compression by using the new dialer, and safely returnsresp(non‑nil whenevererr == nil). Usingrequire.NoErroris consistent with other testenv helpers and appropriate for tests.router/core/websocket.go (1)
93-99: Compression configuration & negotiation wiring look sound
NewWebsocketMiddlewarecorrectly derives handler‑levelcompressionEnabledand a clampedcompressionLevelfromWebSocketConfiguration.Compression, falling back toflate.DefaultCompressionon out‑of‑range values.handleUpgradeRequestwireswsflate.Extensionintows.HTTPUpgrader.Negotiateonly when compression is enabled, and usesext.Accepted()to decide a per‑connectioncompressionNegotiatedflag.- That per‑connection flag is passed into
newWSConnectionWrapper, so compression is only used when actually negotiated with the client; the handler‑levelcompressionLevelis applied uniformly to all compressed connections.This setup keeps configuration, HTTP‑level negotiation, and frame‑level I/O nicely separated and avoids accidentally compressing when the extension wasn’t agreed upon.
Also applies to: 170-183, 356-358, 401-422, 438-439
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
router/core/websocket.go (2)
93-99: Consider allowing compression level 0 (NoCompression).The current validation rejects compression levels outside the range 1-9 and defaults to
flate.DefaultCompression. However,flate.NoCompression(0) is a valid compression level that might be useful for testing or when minimal CPU overhead is desired. Consider adjusting the validation:-if handler.compressionLevel < 1 || handler.compressionLevel > 9 { +if handler.compressionLevel < 0 || handler.compressionLevel > 9 { handler.compressionLevel = flate.DefaultCompression }
285-302: Consider handling the error from flate.NewWriter.On line 289, the error return from
flate.NewWriteris ignored:fw, _ := flate.NewWriter(w, c.compressionLevel)While the compression level is validated during initialization (lines 96-98), defensive coding would suggest handling this error or at minimum adding a comment explaining why it's safe to ignore. If
compressionLevelis somehow corrupted at runtime, this could panic.🔎 Suggested defensive error handling
var buf bytes.Buffer writer := wsflate.NewWriter(&buf, func(w io.Writer) wsflate.Compressor { - fw, _ := flate.NewWriter(w, c.compressionLevel) + fw, err := flate.NewWriter(w, c.compressionLevel) + if err != nil { + // Should never happen as level is validated, but handle defensively + fw, _ = flate.NewWriter(w, flate.DefaultCompression) + } return fw })
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
router-tests/testenv/testenv.gorouter-tests/websocket_test.gorouter/core/websocket.gorouter/pkg/config/config.gorouter/pkg/config/config.schema.jsonrouter/pkg/config/fixtures/full.yamlrouter/pkg/config/testdata/config_defaults.jsonrouter/pkg/config/testdata/config_full.json
🚧 Files skipped from review as they are similar to previous changes (3)
- router/pkg/config/config.go
- router/pkg/config/fixtures/full.yaml
- router-tests/testenv/testenv.go
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-07-21T15:06:36.664Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.
Applied to files:
router/pkg/config/config.schema.jsonrouter/pkg/config/testdata/config_full.json
📚 Learning: 2025-07-21T14:46:34.879Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
Applied to files:
router/pkg/config/config.schema.jsonrouter/pkg/config/testdata/config_full.json
📚 Learning: 2025-10-01T20:39:16.113Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2252
File: router-tests/telemetry/telemetry_test.go:9684-9693
Timestamp: 2025-10-01T20:39:16.113Z
Learning: Repo preference: In router-tests/telemetry/telemetry_test.go, keep strict > 0 assertions for request.operation.*Time (parsingTime, normalizationTime, validationTime, planningTime) in telemetry-related tests; do not relax to >= 0 unless CI flakiness is observed.
Applied to files:
router-tests/websocket_test.go
📚 Learning: 2025-08-28T09:18:10.121Z
Learnt from: endigma
Repo: wundergraph/cosmo PR: 2141
File: router-tests/http_subscriptions_test.go:100-108
Timestamp: 2025-08-28T09:18:10.121Z
Learning: In router-tests/http_subscriptions_test.go heartbeat tests, the message ordering should remain strict with data messages followed by heartbeat messages, as the timing is deterministic and known by design in the Cosmo router implementation.
Applied to files:
router-tests/websocket_test.go
🧬 Code graph analysis (2)
router/core/websocket.go (1)
router/pkg/config/config.go (1)
WebSocketConfiguration(710-727)
router-tests/websocket_test.go (2)
router-tests/testenv/testenv.go (6)
Run(105-122)Config(292-349)Environment(1735-1771)WSWriteJSON(2848-2889)WebSocketMessage(2287-2291)WSReadJSON(2762-2803)router/pkg/config/config.go (2)
Config(1041-1116)WebSocketConfiguration(710-727)
🔇 Additional comments (13)
router/pkg/config/testdata/config_defaults.json (1)
441-445: LGTM! Default configuration is appropriate.The default values are well-chosen:
Enabled: falseensures backward compatibilityLevel: 6provides a balanced trade-off between compression ratio and performancerouter/pkg/config/testdata/config_full.json (1)
821-825: LGTM! Full configuration example looks correct.The configuration correctly demonstrates the compression feature in its enabled state with the default compression level.
router/pkg/config/config.schema.json (2)
563-582: Schema structure and defaults look good.The compression configuration schema is well-defined with:
- Clear, informative descriptions mentioning the permessage-deflate extension
- Appropriate default values (disabled by default for backward compatibility)
- Proper constraints with
additionalProperties: false- Reasonable default compression level of 6 (balanced trade-off)
574-580: Schema correctly constrains compression level range.The schema's
minimum: 1constraint is intentional and properly validated by the implementation. The WebSocket handler (router/core/websocket.go, lines 96-98) explicitly rejects levels below 1:if handler.compressionLevel < 1 || handler.compressionLevel > 9 { handler.compressionLevel = flate.DefaultCompression }While Go's
compress/flatepackage supports level 0 (no compression), the implementation excludes it by design. If users need no compression, they should disable compression entirely via theenabled: falseflag rather than use a level-0 option. The schema correctly enforces this constraint.router-tests/websocket_test.go (3)
2436-2474: LGTM! Validates graceful degradation when compression is disabled on server.This test correctly verifies that when the server has compression disabled, the negotiation fails gracefully and the connection still works without compression. The use of
NotContainsfor the negative assertion is appropriate.
2476-2514: LGTM! Good coverage of custom compression level.The test correctly validates that custom compression levels (level 9 for maximum compression) can be configured and work properly. Using a query with nested fields provides better test coverage for compression behavior with more complex payloads.
2395-2434: Helper function is properly defined and available.The function
InitGraphQLWebSocketConnectionWithCompressionexists inrouter-tests/testenv/testenv.goand returns(*websocket.Conn, *http.Response). The test correctly unpacks the response and accesses the headers viaresp.Header.Get("Sec-WebSocket-Extensions"). The implementation and usage are correct.router/core/websocket.go (6)
170-182: LGTM! Clean addition of compression fields.The compression fields are properly added to the connection wrapper struct and initialized through the constructor.
256-260: LGTM! Clean conditional write path.The write path correctly branches based on compression status and delegates to the appropriate method.
278-283: LGTM! Consistent with WriteText implementation.
401-421: LGTM! Compression negotiation is correctly implemented.The permessage-deflate extension is properly configured with context takeover disabled (safer and simpler), and the negotiation result is correctly checked after upgrade. The
NoContextTakeoversettings ensure that compression state doesn't persist between messages, which simplifies the implementation and avoids potential state management issues.
438-438: LGTM! Compression settings are properly passed to the connection wrapper.The negotiated compression status and level are correctly passed to the connection wrapper constructor.
194-239: Handle or explicitly document WebSocket message fragmentation behavior in compressed read path.The compressed frame reading path (lines 197-231) breaks on the first
ws.OpTextorws.OpBinaryframe without checking for continuation frames. In contrast, the non-compressed path useswsutil.ReadClientText, which implicitly handles message fragmentation. While the graphql-transport-ws protocol typically sends complete messages in single frames without fragmentation, this inconsistency between the two paths creates a potential reliability gap. Either implement continuation frame handling for the compressed path (accumulating payloads until FIN=1) to match the non-compressed behavior, or add a comment explaining why fragmentation is not expected.
1 participant
Following up on an issue I created a couple months ago. #2257 We are in need of this feature to be able to use websockets through cosmo due to the volume of data we are handling.
This adds new websocket compression options to the config and handles the negotiation of the websocket upgrade to use compression if the client requests it.
Summary by CodeRabbit
New Features
Configuration
Tests
✏️ Tip: You can customize this high-level summary in your review settings.
Checklist