fix(server): Skip invalid project keys in batch request (#1093)

Instead of failing a project config batch request during
deserialization, skip invalid keys and serve valid ones. The downstream
Relay assumes missing responses as ProjectState::missing.

CHANGELOG.md

@@ -9,8 +9,10 @@
 - Report proper status codes and error messages when sending invalid JSON payloads to an endpoint with a `X-Sentry-Relay-Signature` header. ([#1090](https://github.com/getsentry/relay/pull/1090))
 
 **Internal**:
+
 - Add the exclusive time of the transaction's root span. ([#1083](https://github.com/getsentry/relay/pull/1083))
 - Add session.status tag to extracted session.duration metric. ([#1087](https://github.com/getsentry/relay/pull/1087))
+- Serve project configs for batched requests where one of the project keys cannot be parsed. ([#1093](https://github.com/getsentry/relay/pull/1093))
 
 ## 21.9.0
 

relay-server/src/actors/project_upstream.rs

@@ -35,13 +35,15 @@ mod _macro {
     }
 }
 
-#[derive(Debug, Deserialize, Serialize)]
+/// A query to retrieve a batch of project states from upstream.
+///
+/// This query does not implement `Deserialize`. To parse the query, use a wrapper that skips
+/// invalid project keys instead of failing the entire batch.
+#[derive(Debug, Serialize)]
 #[serde(rename_all = "camelCase")]
 pub struct GetProjectStates {
     pub public_keys: Vec<ProjectKey>,
-    #[serde(default)]
     pub full_config: bool,
-    #[serde(default)]
     pub no_cache: bool,
 }
 

relay-server/src/endpoints/project_configs.rs

@@ -9,9 +9,9 @@ use relay_common::ProjectKey;
 
 use crate::actors::project::{LimitedProjectState, ProjectState};
 use crate::actors::project_cache::{GetProjectState, ProjectCache};
-use crate::actors::project_upstream::GetProjectStates;
 use crate::extractors::SignedJson;
 use crate::service::ServiceApp;
+use crate::utils::ErrorBoundary;
 
 /// Helper to deserialize the `version` query parameter.
 #[derive(Debug, Deserialize)]
@@ -58,14 +58,31 @@ struct GetProjectStatesResponseWrapper {
     configs: HashMap<ProjectKey, Option<ProjectStateWrapper>>,
 }
 
+/// Request payload of the project config endpoint.
+///
+/// This is a replica of [`GetProjectStates`](crate::actors::project_upstream::GetProjectStates)
+/// which allows skipping invalid project keys.
+#[derive(Debug, Deserialize)]
+#[serde(rename_all = "camelCase")]
+struct GetProjectStatesRequest {
+    public_keys: Vec<ErrorBoundary<ProjectKey>>,
+    #[serde(default)]
+    full_config: bool,
+    #[serde(default)]
+    no_cache: bool,
+}
+
 fn get_project_configs(
-    body: SignedJson<GetProjectStates>,
+    body: SignedJson<GetProjectStatesRequest>,
 ) -> ResponseFuture<Json<GetProjectStatesResponseWrapper>, Error> {
     let relay = body.relay;
     let full = relay.internal && body.inner.full_config;
     let no_cache = body.inner.no_cache;
 
-    let futures = body.inner.public_keys.into_iter().map(move |project_key| {
+    // Skip unparsable public keys. The downstream Relay will consider them `ProjectState::missing`.
+    let valid_keys = body.inner.public_keys.into_iter().filter_map(|e| e.ok());
+
+    let futures = valid_keys.map(move |project_key| {
         let relay = relay.clone();
         ProjectCache::from_registry()
             .send(GetProjectState::new(project_key).no_cache(no_cache))

relay-server/src/utils/error_boundary.rs

@@ -59,6 +59,14 @@ impl<T> ErrorBoundary<T> {
         !self.is_ok()
     }
 
+    #[inline]
+    pub fn ok(self) -> Option<T> {
+        match self {
+            ErrorBoundary::Err(_) => None,
+            ErrorBoundary::Ok(value) => Some(value),
+        }
+    }
+
     #[inline]
     pub fn unwrap_or_else<F>(self, op: F) -> T
     where

tests/integration/test_projectconfigs.py

@@ -2,6 +2,7 @@
 Tests the project_configs endpoint (/api/0/relays/projectconfigs/)
 """
 
+import json
 import uuid
 import pytest
 from collections import namedtuple
@@ -98,7 +99,7 @@ def test_dynamic_relays(mini_sentry, relay, caller, projects):
 def test_invalid_json(mini_sentry, relay):
     relay = relay(mini_sentry, wait_healthcheck=True)
 
-    body = "{}"  # missing the required `public_keys` field
+    body = {}  # missing the required `publicKeys` field
     packed, signature = SecretKey.parse(relay.secret_key).pack(body)
 
     response = relay.post(
@@ -119,7 +120,7 @@ def test_invalid_signature(mini_sentry, relay):
 
     response = relay.post(
         "/api/0/relays/projectconfigs/?version=2",
-        data='{"public_keys":[]}',
+        data='{"publicKeys":[]}',
         headers={
             "X-Sentry-Relay-Id": relay.relay_id,
             "X-Sentry-Relay-Signature": "broken",
@@ -128,3 +129,31 @@ def test_invalid_signature(mini_sentry, relay):
 
     assert response.status_code == 401  # Unauthorized
     assert "signature" in response.text
+
+
+def test_broken_projectkey(mini_sentry, relay):
+    relay = relay(mini_sentry, wait_healthcheck=True)
+    mini_sentry.add_basic_project_config(42)
+    public_key = mini_sentry.get_dsn_public_key(42)
+
+    body = {
+        "publicKeys": [
+            public_key,  # valid
+            "deadbeef",  # wrong length
+            42,  # wrong type
+            "/?$äß000000000000000000000000000",  # invalid characters
+        ]
+    }
+    packed, signature = SecretKey.parse(relay.secret_key).pack(body)
+
+    response = relay.post(
+        "/api/0/relays/projectconfigs/?version=2",
+        data=packed,
+        headers={
+            "X-Sentry-Relay-Id": relay.relay_id,
+            "X-Sentry-Relay-Signature": signature,
+        },
+    )
+
+    assert response.ok
+    assert public_key in response.json()["configs"]