Skip to content

Commit

Permalink
fix: Ensure error observers are correctly added when dealing with err…
Browse files Browse the repository at this point in the history
…ors in the call graph of a middleware of any kind
  • Loading branch information
LukeMathWalker committed Oct 13, 2024
1 parent dab03d2 commit 481f91b
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 26 deletions.
41 changes: 37 additions & 4 deletions libs/pavexc/src/compiler/analyses/call_graph/core_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,6 @@ where
let graph_size_before_transformations = indexes.len();

// For each node, we add the respective transformers, if they have been registered.
let indexes = call_graph.node_indices().collect::<Vec<_>>();
for node_index in indexes {
if transformed_node_indexes.contains(&node_index) {
continue;
Expand Down Expand Up @@ -336,7 +335,7 @@ where
};

// There are two topologies for error handlers:
// - Directly downstream of an error matcher, which in turn has `pavex::Error::new` as a child
// - Directly downstream of the `Err` branch of match, which in turn has `pavex::Error::new` as a child
// - Directly downstream of a `pavex::Error::new` invocations
// We want to find the `pavex::Error::new` node index for this error.
let pavex_error_new_node_index = match call_graph
Expand Down Expand Up @@ -494,11 +493,45 @@ where
assert_eq!(downstream_sources.len(), 1);
downstream_sources[0]
};
Ok(CallGraph {
enforce_invariants(
&call_graph,
error_observer_ids.len(),
component_db,
computation_db,
);

let call_graph = CallGraph {
call_graph,
root_node_index: new_root_index,
root_scope_id,
})
};
Ok(call_graph)
}

fn enforce_invariants(
call_graph: &RawCallGraph,
n_unique_error_observers: usize,
component_db: &ComponentDb,
computation_db: &ComputationDb,
) {
let mut n_error_observers = 0;
let mut n_errors = 0;
for node in call_graph.node_weights() {
if let CallGraphNode::MatchBranching { .. } = node {
n_errors += 1;
continue;
}
if let Some(HydratedComponent::ErrorObserver(_)) =
node.as_hydrated_component(component_db, computation_db)
{
n_error_observers += 1;
};
}
let expected_error_observers = n_errors * n_unique_error_observers;
assert_eq!(
expected_error_observers, n_error_observers,
"There should be {expected_error_observers} error observers in the graph, but we found {n_error_observers}."
)
}

impl CallGraph {
Expand Down
49 changes: 27 additions & 22 deletions libs/pavexc/src/compiler/analyses/components/db/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,57 +197,57 @@ impl ComponentDb {
krate_collection,
diagnostics,
);

// This **must** be invoked after `process_request_handlers` because it relies on
// all request handlers being registered to determine which scopes have error observers.
self_.process_error_observers(
&pavex_error_ref,
self_.process_wrapping_middlewares(
&mut needs_error_handler,
computation_db,
package_graph,
krate_collection,
diagnostics,
);

// We process the backlog of matchers that were not registered during the initial
// registration phase for request handlers.
self_.register_all_matchers(computation_db);
// From this point onwards, all fallible components will automatically get matchers registered.
// All error matchers will be automatically paired with a conversion into `pavex::error::Error` if needed,
// based on the scope they belong to.
self_.autoregister_matchers = true;

self_.process_constructors(
self_.process_pre_processing_middlewares(
&mut needs_error_handler,
computation_db,
framework_item_db,
package_graph,
krate_collection,
diagnostics,
);

self_.process_wrapping_middlewares(
self_.process_post_processing_middlewares(
&mut needs_error_handler,
computation_db,
package_graph,
krate_collection,
diagnostics,
);
self_.process_pre_processing_middlewares(
&mut needs_error_handler,
self_.compute_request2middleware_chain(pavex_noop_wrap_id, computation_db);

// This **must** be invoked after request handlers and middlewares have been
// processed, since it needs to determine which scopes have error observers
// attached to them.
self_.process_error_observers(
&pavex_error_ref,
computation_db,
package_graph,
krate_collection,
diagnostics,
);
self_.process_post_processing_middlewares(

// We process the backlog of matchers that were not registered during the initial
// registration phase for request handlers.
self_.register_all_matchers(computation_db);
// From this point onwards, all fallible components will automatically get matchers registered.
// All error matchers will be automatically paired with a conversion into `pavex::error::Error` if needed,
// based on the scope they belong to.
self_.autoregister_matchers = true;

self_.process_constructors(
&mut needs_error_handler,
computation_db,
framework_item_db,
package_graph,
krate_collection,
diagnostics,
);

self_.compute_request2middleware_chain(pavex_noop_wrap_id, computation_db);
self_.process_error_handlers(
&mut needs_error_handler,
computation_db,
Expand Down Expand Up @@ -850,6 +850,11 @@ impl ComponentDb {
continue;
}
v.push(self.scope_id(component_id));
if let Some(middleware_ids) = self.handler_id2middleware_ids.get(&component_id) {
for middleware_id in middleware_ids {
v.push(self.scope_id(*middleware_id));
}
}
}
self.scope_ids_with_observers = v;
}
Expand Down

0 comments on commit 481f91b

Please sign in to comment.