Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changesets/feat_tylerb_add_result_coercion_errors.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
### Response reformatting and result coercion errors ([PR #8441](https://github.com/apollographql/router/pull/8441))

All subgraph responses are checked and corrected to ensure alignment with the
schema and query. When a misaligned value is returned, it is nullified. Now,
when the feature is enabled, an errors for this nullification are included in
the errors array in the response.

By [@TylerBloom](https://github.com/TylerBloom) in https://github.com/apollographql/router/pull/8441
12 changes: 12 additions & 0 deletions apollo-router/src/configuration/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -732,6 +732,14 @@ pub(crate) struct Supergraph {
/// but request handling will stop immediately when the client connection is closed.
pub(crate) early_cancel: bool,

/// Enable errors generated during response reformatting and result coercion to be returned in
/// responses.
/// Default: false
/// All subgraph responses are checked and corrected to ensure alignment with the schema and
/// query. When enabled, misaligned values will generate errors which are included in errors
/// array in the response.
pub(crate) enable_result_coercion_errors: bool,

/// Log a message if the client closes the connection before the response is sent.
/// Default: false.
pub(crate) experimental_log_on_broken_pipe: bool,
Expand All @@ -758,6 +766,7 @@ impl Supergraph {
generate_query_fragments: Option<bool>,
early_cancel: Option<bool>,
experimental_log_on_broken_pipe: Option<bool>,
insert_result_coercion_errors: Option<bool>,
) -> Self {
Self {
listen: listen.unwrap_or_else(default_graphql_listen),
Expand All @@ -771,6 +780,7 @@ impl Supergraph {
.unwrap_or_else(default_generate_query_fragments),
early_cancel: early_cancel.unwrap_or_default(),
experimental_log_on_broken_pipe: experimental_log_on_broken_pipe.unwrap_or_default(),
enable_result_coercion_errors: insert_result_coercion_errors.unwrap_or_default(),
}
}
}
Expand All @@ -789,6 +799,7 @@ impl Supergraph {
generate_query_fragments: Option<bool>,
early_cancel: Option<bool>,
experimental_log_on_broken_pipe: Option<bool>,
insert_result_coercion_errors: Option<bool>,
) -> Self {
Self {
listen: listen.unwrap_or_else(test_listen),
Expand All @@ -802,6 +813,7 @@ impl Supergraph {
.unwrap_or_else(default_generate_query_fragments),
early_cancel: early_cancel.unwrap_or_default(),
experimental_log_on_broken_pipe: experimental_log_on_broken_pipe.unwrap_or_default(),
enable_result_coercion_errors: insert_result_coercion_errors.unwrap_or_default(),
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10496,6 +10496,11 @@ expression: "&schema"
"description": "abort request handling when the client drops the connection.\nDefault: false.\nWhen set to true, some parts of the request pipeline like telemetry will not work properly,\nbut request handling will stop immediately when the client connection is closed.",
"type": "boolean"
},
"enable_result_coercion_errors": {
"default": false,
"description": "Enable errors generated during response reformatting and result coercion to be returned in\nresponses.\nDefault: false\nAll subgraph responses are checked and corrected to ensure alignment with the schema and\nquery. When enabled, misaligned values will generate errors which are included in errors\narray in the response.",
"type": "boolean"
},
"experimental_log_on_broken_pipe": {
"default": false,
"description": "Log a message if the client closes the connection before the response is sent.\nDefault: false.",
Expand Down Expand Up @@ -12076,6 +12081,7 @@ expression: "&schema"
},
"defer_support": true,
"early_cancel": false,
"enable_result_coercion_errors": false,
"experimental_log_on_broken_pipe": false,
"generate_query_fragments": true,
"introspection": false,
Expand Down
10 changes: 10 additions & 0 deletions apollo-router/src/services/execution/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ use tracing::Span;
use tracing::event;
use tracing_core::Level;

use crate::Configuration;
use crate::apollo_studio_interop::ReferencedEnums;
use crate::apollo_studio_interop::extract_enums_from_response;
use crate::graphql::Error;
Expand Down Expand Up @@ -64,6 +65,7 @@ pub(crate) struct ExecutionService {
pub(crate) schema: Arc<Schema>,
pub(crate) subgraph_schemas: Arc<SubgraphSchemas>,
pub(crate) fetch_service_factory: Arc<FetchServiceFactory>,
pub(crate) configuration: Arc<Configuration>,
/// Subscription config if enabled
subscription_config: Option<SubscriptionConfig>,
apollo_telemetry_config: Option<ApolloTelemetryConfig>,
Expand Down Expand Up @@ -192,6 +194,8 @@ impl ExecutionService {
};

let execution_span = Span::current();
let insert_result_coercion_errors =
self.configuration.supergraph.enable_result_coercion_errors;

let stream = stream
.map(move |mut response: Response| {
Expand Down Expand Up @@ -243,6 +247,7 @@ impl ExecutionService {
&mut nullified_paths,
metrics_ref_mode,
&context,
insert_result_coercion_errors,
response,
)
}))
Expand All @@ -261,6 +266,7 @@ impl ExecutionService {
nullified_paths: &mut Vec<Path>,
metrics_ref_mode: ApolloMetricsReferenceMode,
context: &crate::Context,
insert_result_coercion_errors: bool,
mut response: Response,
) -> Option<Response> {
// responses that would fall under a path that was previously nullified are not sent
Expand Down Expand Up @@ -330,6 +336,7 @@ impl ExecutionService {
variables.clone(),
schema.api_schema(),
variables_set,
insert_result_coercion_errors
);
}

Expand All @@ -340,6 +347,7 @@ impl ExecutionService {
variables.clone(),
schema.api_schema(),
variables_set,
insert_result_coercion_errors
)
,
);
Expand Down Expand Up @@ -635,6 +643,7 @@ pub(crate) struct ExecutionServiceFactory {
pub(crate) subgraph_schemas: Arc<SubgraphSchemas>,
pub(crate) plugins: Arc<Plugins>,
pub(crate) fetch_service_factory: Arc<FetchServiceFactory>,
pub(crate) configuration: Arc<Configuration>,
}

impl ServiceFactory<ExecutionRequest> for ExecutionServiceFactory {
Expand Down Expand Up @@ -663,6 +672,7 @@ impl ServiceFactory<ExecutionRequest> for ExecutionServiceFactory {
subscription_config: subscription_plugin_conf,
subgraph_schemas: self.subgraph_schemas.clone(),
apollo_telemetry_config: apollo_telemetry_conf,
configuration: Arc::clone(&self.configuration),
}
.boxed(),
|acc, (_, e)| e.execution_service(acc),
Expand Down
1 change: 1 addition & 0 deletions apollo-router/src/services/supergraph/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,7 @@ impl PluggableSupergraphServiceBuilder {
subgraph_schemas: query_planner_service.subgraph_schemas(),
plugins: self.plugins.clone(),
fetch_service_factory,
configuration: Arc::clone(&configuration),
};

let execution_service: execution::BoxCloneService = ServiceBuilder::new()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
source: apollo-router/src/services/supergraph/tests.rs
assertion_line: 800
expression: stream.next_response().await.unwrap()
---
{
Expand Down Expand Up @@ -31,7 +32,7 @@ expression: stream.next_response().await.unwrap()
"extensions": {
"valueCompletion": [
{
"message": "Cannot return null for non-nullable field Organization.nonNullId",
"message": "Null value found for non-nullable type ID",
"path": [
"currentUser",
"activeOrganization",
Expand Down Expand Up @@ -68,7 +69,7 @@ expression: stream.next_response().await.unwrap()
"extensions": {
"valueCompletion": [
{
"message": "Cannot return null for non-nullable field Organization.nonNullId",
"message": "Null value found for non-nullable type ID",
"path": [
"currentUser",
"activeOrganization",
Expand Down Expand Up @@ -105,7 +106,7 @@ expression: stream.next_response().await.unwrap()
"extensions": {
"valueCompletion": [
{
"message": "Cannot return null for non-nullable field Organization.nonNullId",
"message": "Null value found for non-nullable type ID",
"path": [
"currentUser",
"activeOrganization",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
source: apollo-router/src/services/supergraph/tests.rs
assertion_line: 1679
expression: deferred
---
{
Expand All @@ -15,7 +16,7 @@ expression: deferred
"extensions": {
"valueCompletion": [
{
"message": "Cannot return null for non-nullable field Organization.nonNullId",
"message": "Null value found for non-nullable type ID",
"path": [
"currentUser",
"org",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
source: apollo-router/src/services/supergraph/tests.rs
assertion_line: 435
expression: response
---
{
Expand All @@ -11,7 +12,7 @@ expression: response
"extensions": {
"valueCompletion": [
{
"message": "Cannot return null for non-nullable field Organization.nonNullId",
"message": "Null value found for non-nullable type ID",
"path": [
"currentUser",
"activeOrganization"
Expand Down
Loading