Skip to content
Open
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
2 changes: 2 additions & 0 deletions apollo-federation/cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,7 @@ fn compose_from_config_inner(
error: SingleFederationError::Internal {
message: format!("Failed to parse YAML config: {}", e),
},
locations: Vec::new(),
}]
})?;

Expand Down Expand Up @@ -337,6 +338,7 @@ fn compose_from_config_inner(
name
),
},
locations: Vec::new(),
}]);
};

Expand Down
53 changes: 45 additions & 8 deletions apollo-federation/src/error/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,17 @@ pub struct SubgraphLocation {

pub type Locations = Vec<SubgraphLocation>;

/// Supergraph coordinates carried on `@inaccessible` API schema errors so composition can attach
/// subgraph source locations (port of JS `inaccessible_elements` / `inaccessible_referencers`
/// extensions and `updateInaccessibleErrorsWithLinkToSubgraphs`). Those locations are surfaced on
/// [`CompositionError::MergeError`] as [`SubgraphLocation`] entries (Rust equivalent of JS
/// `withModifiedErrorNodes`).
#[derive(Clone, Debug, Default)]
pub struct InaccessibleCompositionLinks {
pub elements: Vec<String>,
pub referencers: Vec<String>,
}

pub(crate) trait HasLocations {
fn locations<T: HasMetadata>(&self, subgraph: &Subgraph<T>) -> Locations;
}
Expand All @@ -150,7 +161,10 @@ pub enum CompositionError {
locations: Locations,
},
#[error("{error}")]
MergeError { error: SingleFederationError },
MergeError {
error: SingleFederationError,
locations: Locations,
},
#[error("{error}")]
MergeValidationError { error: SingleFederationError },
#[error("{message}")]
Expand Down Expand Up @@ -498,6 +512,7 @@ impl CompositionError {
| Self::RequiredInputFieldMissingInSomeSubgraph { locations, .. }
| Self::EmptyMergedInputType { locations, .. }
| Self::InvalidFieldSharing { locations, .. }
| Self::MergeError { locations, .. }
| Self::ArgumentDefaultMismatch { locations, .. }
| Self::InputFieldDefaultMismatch { locations, .. } => locations.extend(new_locations),
// Remaining errors do not have an obvious way to appending locations, so we do nothing
Expand All @@ -516,6 +531,7 @@ impl CompositionError {
| Self::RequiredInputFieldMissingInSomeSubgraph { locations, .. }
| Self::EmptyMergedInputType { locations, .. }
| Self::InvalidFieldSharing { locations, .. }
| Self::MergeError { locations, .. }
| Self::ArgumentDefaultMismatch { locations, .. }
| Self::InputFieldDefaultMismatch { locations, .. } => locations,
_ => &[],
Expand Down Expand Up @@ -771,19 +787,40 @@ pub enum SingleFederationError {
#[error("{message}")]
InvalidLinkIdentifier { message: String },
#[error("{message}")]
ReferencedInaccessible { message: String },
ReferencedInaccessible {
message: String,
links: InaccessibleCompositionLinks,
},
#[error("{message}")]
DefaultValueUsesInaccessible { message: String },
DefaultValueUsesInaccessible {
message: String,
links: InaccessibleCompositionLinks,
},
#[error("{message}")]
QueryRootTypeInaccessible { message: String },
QueryRootTypeInaccessible {
message: String,
links: InaccessibleCompositionLinks,
},
#[error("{message}")]
RequiredInaccessible { message: String },
RequiredInaccessible {
message: String,
links: InaccessibleCompositionLinks,
},
#[error("{message}")]
ImplementedByInaccessible { message: String },
ImplementedByInaccessible {
message: String,
links: InaccessibleCompositionLinks,
},
#[error("{message}")]
DisallowedInaccessible { message: String },
DisallowedInaccessible {
message: String,
links: InaccessibleCompositionLinks,
},
#[error("{message}")]
OnlyInaccessibleChildren { message: String },
OnlyInaccessibleChildren {
message: String,
links: InaccessibleCompositionLinks,
},
#[error("{message}")]
RequiredInputFieldMissingInSomeSubgraph { message: String },
#[error("{message}")]
Expand Down
71 changes: 70 additions & 1 deletion apollo-federation/src/link/inaccessible_spec_definition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use apollo_compiler::schema::InputValueDefinition;
use apollo_compiler::schema::Value;

use crate::error::FederationError;
use crate::error::InaccessibleCompositionLinks;
use crate::error::MultipleFederationErrors;
use crate::error::SingleFederationError;
use crate::link::Purpose;
Expand Down Expand Up @@ -249,7 +250,7 @@ fn type_uses_inaccessible(
}

/// Check if a directive uses the @inaccessible directive anywhere in its definition.
fn directive_uses_inaccessible(
pub(crate) fn directive_uses_inaccessible(
inaccessible_directive: &Name,
directive: &DirectiveDefinition,
) -> bool {
Expand Down Expand Up @@ -300,6 +301,10 @@ fn validate_inaccessible_in_default_value(
if input_field_position.is_inaccessible(schema, inaccessible_directive)? {
errors.push(SingleFederationError::DefaultValueUsesInaccessible {
message: format!("Input field `{input_field_position}` is @inaccessible but is used in the default value of `{value_position}`, which is in the API schema."),
links: InaccessibleCompositionLinks {
elements: vec![input_field_position.to_string()],
referencers: vec![value_position.clone()],
},
}.into());
}

Expand Down Expand Up @@ -351,6 +356,10 @@ fn validate_inaccessible_in_default_value(
if enum_value_position.is_inaccessible(schema, inaccessible_directive)? {
errors.push(SingleFederationError::DefaultValueUsesInaccessible {
message: format!("Enum value `{enum_value_position}` is @inaccessible but is used in the default value of `{value_position}`, which is in the API schema."),
links: InaccessibleCompositionLinks {
elements: vec![enum_value_position.to_string()],
referencers: vec![value_position.clone()],
},
}.into());
}
}
Expand Down Expand Up @@ -380,6 +389,10 @@ fn validate_inaccessible_in_arguments(
};
errors.push(SingleFederationError::RequiredInaccessible {
message: format!("Argument `{usage_position}({arg_name}:)` is @inaccessible but is a required argument of its {kind}."),
links: InaccessibleCompositionLinks {
elements: vec![format!("{usage_position}({arg_name}:)")],
referencers: vec![],
},
}.into());
}

Expand Down Expand Up @@ -438,6 +451,10 @@ fn validate_inaccessible_in_fields(
errors.push(
SingleFederationError::ImplementedByInaccessible {
message: format!("Field `{type_position}.{field_name}` is @inaccessible but implements the interface field `{super_position}`, which is in the API schema."),
links: InaccessibleCompositionLinks {
elements: vec![format!("{type_position}.{field_name}")],
referencers: vec![super_position.to_string()],
},
}
.into(),
);
Expand Down Expand Up @@ -475,6 +492,12 @@ fn validate_inaccessible_in_fields(
for accessible_reference in accessible_super_references {
errors.push(SingleFederationError::ImplementedByInaccessible {
message: format!("Argument `{type_position}.{field_name}({arg_name}:)` is @inaccessible but implements the interface argument `{accessible_reference}` which is in the API schema."),
links: InaccessibleCompositionLinks {
elements: vec![format!(
"{type_position}.{field_name}({arg_name}:)"
)],
referencers: vec![accessible_reference.to_string()],
},
}.into());
}
} else if arg.is_required() {
Expand Down Expand Up @@ -520,6 +543,12 @@ fn validate_inaccessible_in_fields(
for inaccessible_reference in inaccessible_super_references {
errors.push(SingleFederationError::RequiredInaccessible {
message: format!("Argument `{inaccessible_reference}` is @inaccessible but is implemented by the argument `{type_position}.{field_name}({arg_name}:)` which is in the API schema."),
links: InaccessibleCompositionLinks {
elements: vec![inaccessible_reference.to_string()],
referencers: vec![format!(
"{type_position}.{field_name}({arg_name}:)"
)],
},
}.into());
}
}
Expand Down Expand Up @@ -549,6 +578,10 @@ fn validate_inaccessible_in_fields(
if has_inaccessible_field && !has_accessible_field {
errors.push(SingleFederationError::OnlyInaccessibleChildren {
message: format!("Type `{type_position}` is in the API schema but all of its members are @inaccessible."),
links: InaccessibleCompositionLinks {
elements: vec![type_position.to_string()],
referencers: vec![],
},
}.into());
}

Expand Down Expand Up @@ -751,6 +784,10 @@ fn validate_inaccessible_type(
if !$ref.is_inaccessible(schema, inaccessible_directive)? {
errors.push(SingleFederationError::ReferencedInaccessible {
message: format!("Type `{}` is @inaccessible but is referenced by `{}`, which is in the API schema.", $ty, $ref),
links: InaccessibleCompositionLinks {
elements: vec![$ty.to_string()],
referencers: vec![$ref.to_string()],
},
}.into())
}
}
Expand Down Expand Up @@ -805,6 +842,10 @@ fn validate_inaccessible_type(
{
errors.push(SingleFederationError::QueryRootTypeInaccessible {
message: format!("Type `{position}` is @inaccessible but is the query root type, which must be in the API schema."),
links: InaccessibleCompositionLinks {
elements: vec![position.to_string()],
referencers: vec![],
},
}.into());
}
check_inaccessible_referencers!(
Expand Down Expand Up @@ -899,6 +940,10 @@ fn validate_inaccessible(
message: format!(
"Core feature type `{position}` cannot use @inaccessible."
),
links: InaccessibleCompositionLinks {
elements: vec![position.to_string()],
referencers: vec![],
},
}
.into(),
)
Expand Down Expand Up @@ -947,6 +992,10 @@ fn validate_inaccessible(
if field_inaccessible && field.is_required() {
errors.push(SingleFederationError::RequiredInaccessible{
message: format!("Input field `{position}` is @inaccessible but is a required input field of its type."),
links: InaccessibleCompositionLinks {
elements: vec![input_object_position.field(field.name.clone()).to_string()],
referencers: vec![],
},
}.into());
}

Expand All @@ -970,6 +1019,10 @@ fn validate_inaccessible(
if has_inaccessible_field && !has_accessible_field {
errors.push(SingleFederationError::OnlyInaccessibleChildren {
message: format!("Type `{position}` is in the API schema but all of its input fields are @inaccessible."),
links: InaccessibleCompositionLinks {
elements: vec![position.to_string()],
referencers: vec![],
},
}.into());
}
}
Expand All @@ -985,6 +1038,10 @@ fn validate_inaccessible(
if !any_accessible_member {
errors.push(SingleFederationError::OnlyInaccessibleChildren {
message: format!("Type `{position}` is in the API schema but all of its members are @inaccessible."),
links: InaccessibleCompositionLinks {
elements: vec![position.to_string()],
referencers: vec![],
},
}.into());
}
}
Expand All @@ -1001,6 +1058,10 @@ fn validate_inaccessible(
if has_inaccessible_value && !has_accessible_value {
errors.push(SingleFederationError::OnlyInaccessibleChildren {
message: format!("Type `{enum_position}` is in the API schema but all of its members are @inaccessible."),
links: InaccessibleCompositionLinks {
elements: vec![enum_position.to_string()],
referencers: vec![],
},
}.into());
}
}
Expand Down Expand Up @@ -1028,6 +1089,10 @@ fn validate_inaccessible(
message: format!(
"Core feature directive `{position}` cannot use @inaccessible.",
),
links: InaccessibleCompositionLinks {
elements: vec![position.to_string()],
referencers: vec![],
},
}
.into(),
);
Expand All @@ -1042,6 +1107,10 @@ fn validate_inaccessible(
.join(", ");
errors.push(SingleFederationError::DisallowedInaccessible {
message: format!("Directive `{position}` cannot use @inaccessible because it may be applied to these type-system locations: {type_system_locations}"),
links: InaccessibleCompositionLinks {
elements: vec![position.to_string()],
referencers: vec![],
},
}.into());
}
} else {
Expand Down
15 changes: 11 additions & 4 deletions apollo-federation/src/merger/merger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ impl Merger {
hints,
});
}
match Self::validate_supergraph_schema(self.merged) {
match Self::validate_supergraph_schema(self.merged.clone(), &self.subgraphs) {
Ok(supergraph) => Ok(MergeResult {
supergraph: Some(supergraph),
errors: Vec::default(),
Expand All @@ -577,6 +577,7 @@ impl Merger {
/// computed.
fn validate_supergraph_schema(
merged: FederationSchema,
subgraphs: &[Subgraph<Validated>],
) -> Result<ValidFederationSchema, Vec<CompositionError>> {
// TODO: Errors thrown by the `validate` below are likely to be confusing for users,
// because they refer to a document they don't know about (the merged-but-not-returned
Expand All @@ -595,8 +596,11 @@ impl Merger {
// other errors in theory, but if there are, better to find it now rather than later).
api_schema::to_api_schema(supergraph_schema.clone(), Default::default()).map_err(
|err| {
// TODO: port `updateInaccessibleErrorsWithLinkToSubgraphs` from JS (FED-882)
Self::convert_to_merge_errors(err)
super::supergraph_coordinate::update_inaccessible_errors_with_link_to_subgraphs(
&supergraph_schema,
subgraphs,
err,
)
},
)?;

Expand All @@ -608,7 +612,10 @@ impl Merger {
error
.into_errors()
.into_iter()
.map(|e| CompositionError::MergeError { error: e })
.map(|e| CompositionError::MergeError {
error: e,
locations: Vec::new(),
})
.collect()
}

Expand Down
1 change: 1 addition & 0 deletions apollo-federation/src/merger/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,4 @@ mod merge_interface;
mod merge_links;
mod merge_type;
mod merge_union;
mod supergraph_coordinate;
Loading
Loading