ingest-router: Simplify handler type #92

lynnagara · 2025-12-18T21:14:33Z

This change does a few things:

simplifies the handler trait so it is no longer generic, and enables additional handlers to be added
adds a "health" handler stub -- this doesn't do anything yet except demonstrates that it is now possible
to now add more than one handler.
finishes the merge responses implementation in the project configs handler

The generics solution does not work well for our use case since it requires the concrete type to be known at compile time. In our case the router picks the handler dynamically at runtime based on inspecting the incoming route.

With Arc<dyn Handler>, we can more easily dyanamically dispatch to the appropriate handler based on the matched route.

Each handler receives the raw requests/responses and is responsible for splitting and merging them as needed.

This chnage simplifies the handler trait so it is no longer generic. The generics solution does not work well for our use case since generics requires the concrete type to be known at compile time, however in our case the router picks the handler dynamically at runtime based on inspecting the incoming route. With Arc<dyn Handler>, we can more easily dyanamically dispatch to the appropriate handler based on the matched route. Each handler receives the raw requests/responses and is responsible for splitting and merging them as needed.

ingest-router/src/lib.rs

+                    let (split, _metadata) =
+                        handler.split_request(handler_req, &cells).await.unwrap();


ingest-router/src/api/health.rs

+        _request: Request<HandlerBody>,
+        _cells: &Cells,
+    ) -> Result<(Vec<(CellId, Request<HandlerBody>)>, SplitMetadata), IngestRouterError> {


lynnagara · 2025-12-19T19:04:16Z

ingest-router/src/project_config/handler.rs

+        // Downcast metadata to our specific type
+        if let Ok(project_metadata) = metadata.downcast::<ProjectConfigsMetadata>() {
+            merged.pending_keys.extend(*project_metadata);
+        }


this is kind of the downside of this approach -- it's probably worth attempting to write a macro or enum here to avoid this need to manually downcast to the real type.

added a TODO for later

ingest-router/src/project_config/handler.rs

        assert!(pending.contains(&serde_json::json!("key_routing_failed")));
        assert!(pending.contains(&serde_json::json!("key_from_failed_cell1")));


ingest-router/src/project_config/handler.rs

+        let pending_from_split: ProjectConfigsMetadata = vec![
            "key_routing_failed".to_string(),
            "key_from_failed_cell1".to_string(),
            "key_from_failed_cell2".to_string(),
        ];


lynnagara · 2025-12-22T23:49:12Z

ingest-router/src/project_config/handler.rs

+#[derive(Default, Debug)]
+struct ProjectConfigsMetadata {
+    // keys that are assigned to a cell
+    cell_to_keys: HashMap<CellId, Vec<String>>,


this has to be added to the metadata -- if the request to a cell fails for any reason, we can look it up on this map and add the keys to pending during the merge phase

lynnagara · 2025-12-22T23:50:19Z

ingest-router/src/project_config/handler.rs

-        // TODO: The current implementation does not handle errors from the results
-        // parameter. The edge case to be handled are if any of the upstreams failed
-        // to return a response for whatever reason. In scenarios like this, the
-        // executor needs to provide all the project config keys which failed to
-        // resolve on the upstream. We would need to add those project keys to the
-        // pending response.


this is done now -- the split phase returns the cell to keys map, which allows us to figure out which keys are the failed ones from this function

lynnagara · 2025-12-22T23:50:58Z

ingest-router/src/lib.rs


        let response = service.call(request).await.unwrap();

+        // TODO: call the scripts/mock_relay_api.py server and validate the response


added a note for later -- we should add a true end to end test here

lynnagara · 2025-12-22T23:53:27Z

ingest-router/src/api/utils.rs

+pub async fn deserialize_body<T: DeserializeOwned>(
+    body: HandlerBody,
+) -> Result<T, IngestRouterError> {
+    let bytes = body.collect().await?.to_bytes();
+    serde_json::from_slice(&bytes).map_err(|e| IngestRouterError::RequestBodyError(e.to_string()))
+}
+
+/// Serializes a value to a JSON body.
+pub fn serialize_to_body<T: Serialize>(value: &T) -> Result<HandlerBody, IngestRouterError> {
+    let bytes = serde_json::to_vec(value).map(Bytes::from)?;
+    Ok(Full::new(bytes).map_err(|e| match e {}).boxed())
+}


moved these functions out of the project config implementation into utils as we can reuse them for any kind of json endpoint

lynnagara · 2025-12-22T23:54:18Z

ingest-router/src/api/utils.rs

+}
+
+/// Common header normalization for all requests and responses.
+pub fn normalize_headers(headers: &mut HeaderMap, version: Version) -> &mut HeaderMap {


pulled this out as well, as we can reuse it for any relay endpoint in both directions - requests sent upstream and responses back

ingest-router/src/project_config/handler.rs

+            if let Ok(parsed) = deserialize_body::<ProjectConfigsResponse>(body).await {
+                merged.project_configs.extend(parsed.project_configs);
+                merged.extra_fields.extend(parsed.extra_fields);
+                merged.pending_keys.extend(parsed.pending_keys);
+            }


lynnagara · 2025-12-23T00:01:42Z

ingest-router/src/locale.rs

    /// Get the cells for a specific locale
-    pub fn get_cells(&self, locale: &str) -> Option<&Cells> {
-        self.locale_to_cells.get(locale)
+    pub fn get_cells(&self, locale: &str) -> Option<Arc<Cells>> {


Switched to Arc<Cells> since we pass Cells to a bunch of places now - and it's easier for the data to be owned

lynnagara · 2025-12-23T22:07:27Z

ingest-router/src/project_config/protocol.rs

-    }
-}
-
-#[cfg(test)]


removed these tests since ProjectConfigsRequest and ProjectConfigsResponse are barebones structs now -- i don't think there's too much value in testing builtin rust structures

lynnagara requested a review from a team as a code owner December 18, 2025 21:14

lynnagara added 4 commits December 18, 2025 13:16

.

23d2c80

make more stuff generic

f94c2ec

it's used now

83ea0d5

.

b4df820

sentry bot reviewed Dec 18, 2025

View reviewed changes

ingest-router/src/lib.rs Outdated

Comment on lines 68 to 69

let (split, _metadata) =

handler.split_request(handler_req, &cells).await.unwrap();

This comment was marked as outdated.

Sign in to view

lynnagara added 2 commits December 19, 2025 10:35

fix unwrap

9d8824c

.

a956d70

sentry bot reviewed Dec 19, 2025

View reviewed changes

ingest-router/src/api/health.rs

Comment on lines +15 to +17

_request: Request<HandlerBody>,

_cells: &Cells,

) -> Result<(Vec<(CellId, Request<HandlerBody>)>, SplitMetadata), IngestRouterError> {

This comment was marked as outdated.

Sign in to view

lynnagara added 2 commits December 19, 2025 10:38

revert moving comment

139acf0

.

b84d372

lynnagara commented Dec 19, 2025

View reviewed changes

lynnagara added 2 commits December 19, 2025 11:48

remove test for deleted func

64ca3d4

merge responses

f26e5ac

sentry bot reviewed Dec 19, 2025

View reviewed changes

lynnagara added 3 commits December 19, 2025 15:13

more refactor

db100bb

finish implementing merge_responses

86f3804

refactor tests

a1d0f09

lynnagara commented Dec 22, 2025

View reviewed changes

add todo

95b7a70

sentry bot reviewed Dec 23, 2025

View reviewed changes

lynnagara commented Dec 23, 2025

View reviewed changes

lynnagara added 2 commits December 22, 2025 16:11

add error msg

4e856e3

remove commented tests

da9ab40

lynnagara commented Dec 23, 2025

View reviewed changes

lynnagara mentioned this pull request Dec 23, 2025

ingest-router: add executor and actually return results to client #93

Open

		let (split, _metadata) =
		handler.split_request(handler_req, &cells).await.unwrap();

		assert!(pending.contains(&serde_json::json!("key_routing_failed")));
		assert!(pending.contains(&serde_json::json!("key_from_failed_cell1")));


		let response = service.call(request).await.unwrap();

		// TODO: call the scripts/mock_relay_api.py server and validate the response

Uh oh!

ingest-router: Simplify handler type #92

Are you sure you want to change the base?

ingest-router: Simplify handler type #92

Uh oh!

Conversation

lynnagara commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lynnagara commented Dec 18, 2025 •

edited

Loading