router icon indicating copy to clipboard operation
router copied to clipboard

fix(coprocessor): improve handling of invalid GraphQL responses with conditional validation

Open BrynCooke opened this issue 5 months ago • 1 comments

Task

Fix handling of invalid GraphQL responses returned from coprocessors, particularly when used with subscriptions. Added conditional response validation and improved testing to ensure correctness.

Why this change is needed

The router was creating invalid GraphQL responses internally, especially when subscriptions terminate. When a coprocessor is configured, it validates all responses for correctness, causing errors to be logged when the router generates invalid internal responses. This affects the reliability of subscription workflows with coprocessors.

How it was tackled

Coprocessor response validation improvements

  • Added conditional response validation that only validates coprocessor responses when the incoming payload was valid
  • Added response_validation configuration option (defaults to true) for backward compatibility
  • Improved handling of invalid responses to prevent validation failures on already-invalid data

Subscription integration tests

  • WebSocket passthrough tests: Mock server using axum with GraphQL-WS protocol, tests connection lifecycle and streaming behavior
  • HTTP callback tests: Mock callback and subgraph servers, tests subscription lifecycle through HTTP callbacks with error scenarios
  • Tests cover normal operation, error payloads, pure error responses, and coprocessor integration
  • Added consistent event verification and error logging detection

Documentation updates

  • Added comprehensive documentation for the response_validation configuration option
  • Documented use cases, behavior differences, and migration guidance

Technical details

  • Conditional validation prevents failures on responses that were already invalid before coprocessor processing
  • ENABLE_CONDITIONAL_VALIDATION const controls the new behavior (set to true)
  • was_incoming_payload_valid() function checks if original response was minimally valid
  • Added comprehensive test coverage for all coprocessor stages with validation enabled/disabled

Suggested review strategy

  • Examine conditional validation logic in handle_graphql_response() function in coprocessor/mod.rs
  • Review was_incoming_payload_valid() implementation for payload validity checking
  • Check callback server HTTP protocol implementation in subscription test files
  • Validate test coverage for coprocessor validation scenarios across all pipeline stages
  • Review documentation accuracy for the new response_validation configuration option

<!-- start metadata --> EOF < /dev/null

BrynCooke avatar Jun 19 '25 13:06 BrynCooke

✅ Docs preview has no changes

The preview was not built because there were no changes.

Build ID: e9f1abe09469fcd64e80a212

apollo-librarian[bot] avatar Jun 19 '25 13:06 apollo-librarian[bot]

@mergifyio backport dev

abernix avatar Jun 26 '25 08:06 abernix

backport dev

❌ No backport have been created

  • Backport to branch dev failed

Git reported the following error:

remote: The 'apollographql' organization has enabled or enforced SAML SSO.
remote: To access this repository, you must re-authorize the GitHub App 'Mergify'.
fatal: unable to access 'https://github.com/apollographql/router/': The requested URL returned error: 403

mergify[bot] avatar Jun 26 '25 08:06 mergify[bot]

I'm not really happy with the docs yet, and as they would be available immediately after landing this PR, I've just deleted them. I'll follow up with a PR that re-introduces the docs changes and apply some edits at that point.

goto-bus-stop avatar Jun 26 '25 15:06 goto-bus-stop

@mergifyio backport dev

abernix avatar Jun 27 '25 11:06 abernix

backport dev

❌ No backport have been created

  • Backport to branch dev failed

Git reported the following error:

remote: The 'apollographql' organization has enabled or enforced SAML SSO.
remote: To access this repository, you must re-authorize the GitHub App 'Mergify'.
fatal: unable to access 'https://github.com/apollographql/router/': The requested URL returned error: 403

mergify[bot] avatar Jun 27 '25 11:06 mergify[bot]

backport dev

❌ No backport have been created

  • Backport to branch dev failed

Git reported the following error:

remote: The 'apollographql' organization has enabled or enforced SAML SSO.
remote: To access this repository, you must re-authorize the GitHub App 'Mergify'.
fatal: unable to access 'https://github.com/apollographql/router/': The requested URL returned error: 403

mergify[bot] avatar Jun 27 '25 11:06 mergify[bot]

@mergifyio backport dev

abernix avatar Jun 27 '25 11:06 abernix

backport dev

❌ No backport have been created

  • Backport to branch dev failed

Git reported the following error:

remote: The 'apollographql' organization has enabled or enforced SAML SSO.
remote: To access this repository, you must re-authorize the GitHub App 'Mergify'.
fatal: unable to access 'https://github.com/apollographql/router/': The requested URL returned error: 403

mergify[bot] avatar Jun 27 '25 11:06 mergify[bot]

@mergifyio backport dev

abernix avatar Jun 27 '25 12:06 abernix

backport dev

❌ No backport have been created

  • Backport to branch dev failed

Git reported the following error:

remote: The 'apollographql' organization has enabled or enforced SAML SSO.
remote: To access this repository, you must re-authorize the GitHub App 'Mergify'.
fatal: unable to access 'https://github.com/apollographql/router/': The requested URL returned error: 403

mergify[bot] avatar Jun 27 '25 12:06 mergify[bot]

@mergifyio backport dev

abernix avatar Jun 30 '25 11:06 abernix

backport dev

✅ Backports have been created

mergify[bot] avatar Jun 30 '25 11:06 mergify[bot]