Skip to content

Commit

Permalink
chore(federation): add format-only error macro (#6236)
Browse files Browse the repository at this point in the history
  • Loading branch information
@goto-bus-stop
goto-bus-stop authored Nov 7, 2024
1 parent 1e5e703 commit 0189a16
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 51 deletions.
49 changes: 47 additions & 2 deletions apollo-federation/src/error/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,44 @@ use lazy_static::lazy_static;

use crate::subgraph::spec::FederationSpecError;

/// Break out of the current function, returning an internal error.
/// Create an internal error.
///
/// # Example
/// ```rust
/// use apollo_federation::internal_error;
/// use apollo_federation::error::FederationError;
/// # fn may_be_none() -> Option<()> { None }
///
/// const NAME: &str = "the thing";
/// let result: Result<(), FederationError> = may_be_none()
/// .ok_or_else(|| internal_error!("Expected {NAME} to be Some"));
/// ```
#[macro_export]
macro_rules! internal_error {
( $( $arg:tt )+ ) => {
return Err($crate::error::FederationError::internal(format!( $( $arg )+ )).into());
$crate::error::FederationError::internal(format!( $( $arg )+ ))
}
}

/// Break out of the current function, returning an internal error.
///
/// # Example
/// ```rust
/// use apollo_federation::bail;
/// use apollo_federation::error::FederationError;
/// # fn may_be_none() -> Option<()> { None }
///
/// fn example() -> Result<(), FederationError> {
/// bail!("Something went horribly wrong");
/// unreachable!()
/// }
/// #
/// # _ = example();
/// ```
#[macro_export]
macro_rules! bail {
( $( $arg:tt )+ ) => {
return Err($crate::internal_error!( $( $arg )+ ).into());
}
}

Expand All @@ -26,6 +59,18 @@ macro_rules! internal_error {
///
/// Treat this as an assertion. It must only be used for conditions that *should never happen*
/// in normal operation.
///
/// # Example
/// ```rust,no_run
/// use apollo_federation::ensure;
/// use apollo_federation::error::FederationError;
/// # fn may_be_none() -> Option<()> { None }
///
/// fn example() -> Result<(), FederationError> {
/// ensure!(1 == 0, "Something went horribly wrong");
/// unreachable!()
/// }
/// ```
#[macro_export]
macro_rules! ensure {
( $expr:expr, $( $arg:tt )+ ) => {
Expand Down
14 changes: 7 additions & 7 deletions apollo-federation/src/operation/merging.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ use super::NamedFragments;
use super::Selection;
use super::SelectionSet;
use super::SelectionValue;
use crate::bail;
use crate::ensure;
use crate::error::FederationError;
use crate::internal_error;

impl<'a> FieldSelectionValue<'a> {
/// Merges the given field selections into this one.
Expand Down Expand Up @@ -50,14 +50,14 @@ impl<'a> FieldSelectionValue<'a> {
);
if self.get().selection_set.is_some() {
let Some(other_selection_set) = &other.selection_set else {
internal_error!(
bail!(
"Field \"{}\" has composite type but not a selection set",
other_field.field_position,
);
};
selection_sets.push(other_selection_set);
} else if other.selection_set.is_some() {
internal_error!(
bail!(
"Field \"{}\" has non-composite type but also has a selection set",
other_field.field_position,
);
Expand Down Expand Up @@ -187,7 +187,7 @@ impl SelectionSet {
selection_map::Entry::Occupied(existing) => match existing.get() {
Selection::Field(self_field_selection) => {
let Selection::Field(other_field_selection) = other_selection else {
internal_error!(
bail!(
"Field selection key for field \"{}\" references non-field selection",
self_field_selection.field.field_position,
);
Expand All @@ -201,7 +201,7 @@ impl SelectionSet {
let Selection::FragmentSpread(other_fragment_spread_selection) =
other_selection
else {
internal_error!(
bail!(
"Fragment spread selection key for fragment \"{}\" references non-field selection",
self_fragment_spread_selection.spread.fragment_name,
);
Expand All @@ -215,7 +215,7 @@ impl SelectionSet {
let Selection::InlineFragment(other_inline_fragment_selection) =
other_selection
else {
internal_error!(
bail!(
"Inline fragment selection key under parent type \"{}\" {}references non-field selection",
self_inline_fragment_selection.inline_fragment.parent_type_position,
self_inline_fragment_selection.inline_fragment.type_condition_position.clone()
Expand Down Expand Up @@ -368,7 +368,7 @@ pub(crate) fn merge_selection_sets(
mut selection_sets: Vec<SelectionSet>,
) -> Result<SelectionSet, FederationError> {
let Some((first, remainder)) = selection_sets.split_first_mut() else {
internal_error!("merge_selection_sets(): must have at least one selection set");
bail!("merge_selection_sets(): must have at least one selection set");
};
first.merge_into(remainder.iter())?;

Expand Down
57 changes: 20 additions & 37 deletions apollo-federation/src/query_graph/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use petgraph::Direction;

use crate::error::FederationError;
use crate::error::SingleFederationError;
use crate::internal_error;
use crate::operation::Field;
use crate::operation::InlineFragment;
use crate::operation::SelectionSet;
Expand Down Expand Up @@ -368,39 +369,27 @@ impl QueryGraph {
}

pub(crate) fn node_weight(&self, node: NodeIndex) -> Result<&QueryGraphNode, FederationError> {
self.graph.node_weight(node).ok_or_else(|| {
SingleFederationError::Internal {
message: "Node unexpectedly missing".to_owned(),
}
.into()
})
self.graph
.node_weight(node)
.ok_or_else(|| internal_error!("Node unexpectedly missing"))
}

fn node_weight_mut(&mut self, node: NodeIndex) -> Result<&mut QueryGraphNode, FederationError> {
self.graph.node_weight_mut(node).ok_or_else(|| {
SingleFederationError::Internal {
message: "Node unexpectedly missing".to_owned(),
}
.into()
})
self.graph
.node_weight_mut(node)
.ok_or_else(|| internal_error!("Node unexpectedly missing"))
}

pub(crate) fn edge_weight(&self, edge: EdgeIndex) -> Result<&QueryGraphEdge, FederationError> {
self.graph.edge_weight(edge).ok_or_else(|| {
SingleFederationError::Internal {
message: "Edge unexpectedly missing".to_owned(),
}
.into()
})
self.graph
.edge_weight(edge)
.ok_or_else(|| internal_error!("Edge unexpectedly missing"))
}

fn edge_weight_mut(&mut self, edge: EdgeIndex) -> Result<&mut QueryGraphEdge, FederationError> {
self.graph.edge_weight_mut(edge).ok_or_else(|| {
SingleFederationError::Internal {
message: "Edge unexpectedly missing".to_owned(),
}
.into()
})
self.graph
.edge_weight_mut(edge)
.ok_or_else(|| internal_error!("Edge unexpectedly missing"))
}

pub(crate) fn edge_head_weight(
Expand All @@ -415,12 +404,9 @@ impl QueryGraph {
&self,
edge: EdgeIndex,
) -> Result<(NodeIndex, NodeIndex), FederationError> {
self.graph.edge_endpoints(edge).ok_or_else(|| {
SingleFederationError::Internal {
message: "Edge unexpectedly missing".to_owned(),
}
.into()
})
self.graph
.edge_endpoints(edge)
.ok_or_else(|| internal_error!("Edge unexpectedly missing"))
}

fn schema(&self) -> Result<&ValidFederationSchema, FederationError> {
Expand All @@ -431,12 +417,9 @@ impl QueryGraph {
&self,
source: &str,
) -> Result<&ValidFederationSchema, FederationError> {
self.sources.get(source).ok_or_else(|| {
SingleFederationError::Internal {
message: "Schema unexpectedly missing".to_owned(),
}
.into()
})
self.sources
.get(source)
.ok_or_else(|| internal_error!(r#"Schema for "{source}" unexpectedly missing"#))
}

pub(crate) fn subgraph_schemas(&self) -> &IndexMap<Arc<str>, ValidFederationSchema> {
Expand All @@ -454,7 +437,7 @@ impl QueryGraph {
) -> Result<&IndexSet<NodeIndex>, FederationError> {
self.types_to_nodes()?
.get(name)
.ok_or_else(|| FederationError::internal("No nodes unexpectedly found for type"))
.ok_or_else(|| internal_error!("No nodes unexpectedly found for type"))
}

pub(crate) fn types_to_nodes(
Expand Down
10 changes: 5 additions & 5 deletions apollo-federation/src/query_plan/conditions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ use apollo_compiler::Node;
use indexmap::map::Entry;
use serde::Serialize;

use crate::bail;
use crate::error::FederationError;
use crate::internal_error;
use crate::operation::DirectiveList;
use crate::operation::NamedFragments;
use crate::operation::Selection;
Expand Down Expand Up @@ -122,7 +122,7 @@ impl Conditions {

if let Some(skip) = directives.get("skip") {
let Some(value) = skip.specified_argument_by_name("if") else {
internal_error!("missing @skip(if:) argument");
bail!("missing @skip(if:) argument");
};

match value.as_ref() {
Expand All @@ -134,14 +134,14 @@ impl Conditions {
variables.insert(name.clone(), ConditionKind::Skip);
}
_ => {
internal_error!("expected boolean or variable `if` argument, got {value}");
bail!("expected boolean or variable `if` argument, got {value}");
}
}
}

if let Some(include) = directives.get("include") {
let Some(value) = include.specified_argument_by_name("if") else {
internal_error!("missing @include(if:) argument");
bail!("missing @include(if:) argument");
};

match value.as_ref() {
Expand All @@ -159,7 +159,7 @@ impl Conditions {
}
}
_ => {
internal_error!("expected boolean or variable `if` argument, got {value}");
bail!("expected boolean or variable `if` argument, got {value}");
}
}
}
Expand Down

0 comments on commit 0189a16

Please sign in to comment.