From 7236074abf5d2b92db61d53cf22a146ac025782d Mon Sep 17 00:00:00 2001 From: Prajjwal kumar Date: Sun, 30 Aug 2026 20:46:21 +0530 Subject: [PATCH 1/2] fix(routing): treat a profile with no rules like one with none active routing_evaluate answered ActiveRoutingAlgorithmNotFound as a success only when the profile already had at least one rule, on the reasoning that a deactivated profile is stating a choice while a profile the engine has never been given any rules for is a different situation. That distinction does not survive contact with the caller: Hyperswitch routes by the fallback it supplied either way, so the second case only ever produced a failure the caller could not act on. Both now take the same 200 with the caller's fallback, which makes the engine correct for any caller that reaches it without an active rule rather than only for the ones Hyperswitch happens to filter first. It also removes the extra per-request lookup profile_has_any_routing_rule needed, which ran on every evaluate a ruleless profile made -- exactly the profiles least able to justify the query. A profile still awaiting migration is reported by the caller, which warns when a cut-over profile gets an empty result. Co-Authored-By: Claude Opus 5 --- src/euclid/handlers/routing_rules.rs | 84 +++------------------------- 1 file changed, 9 insertions(+), 75 deletions(-) diff --git a/src/euclid/handlers/routing_rules.rs b/src/euclid/handlers/routing_rules.rs index 8b7fc74c..e3246fb6 100644 --- a/src/euclid/handlers/routing_rules.rs +++ b/src/euclid/handlers/routing_rules.rs @@ -29,9 +29,8 @@ use crate::euclid::{ }; use crate::generics::MeshError; use crate::{euclid::types::RoutingAlgorithm, logger, metrics}; -use async_bb8_diesel::AsyncRunQueryDsl; use axum::{extract::Path, response::IntoResponse, Json}; -use diesel::{associations::HasTable, BoolExpressionMethods, ExpressionMethods, QueryDsl}; +use diesel::{associations::HasTable, BoolExpressionMethods, ExpressionMethods}; use error_stack::ResultExt; use crate::app::get_tenant_app_state; @@ -622,69 +621,6 @@ async fn no_active_algorithm_response( response } -/// Whether the profile has any routing rule at all, scoped the same way the active-mapper -/// lookup is. Separates a profile that deactivated its rules from one the engine has never -/// been given any -- the two are indistinguishable from the mapper miss alone, but callers -/// must treat them oppositely: the first is an authoritative "no rule", the second means the -/// engine simply has nothing to say about this profile. -async fn profile_has_any_routing_rule( - state: &crate::app::TenantAppState, - created_by: &str, - algorithm_for: Option<&str>, -) -> bool { - let conn = match state.db.get_conn().await { - Ok(conn) => conn, - Err(error) => { - logger::error!( - ?error, - created_by = %created_by, - "routing_evaluate: no connection to check for existing routing rules" - ); - return false; - } - }; - - // A deactivated profile has no cache entry and no active mapper row, so this runs on every - // evaluate it makes. Selecting one id keeps it off the row itself, whose `algorithm_data` - // holds the whole rule program. - let existing: Result, _> = match algorithm_for { - Some(algorithm_for) => { - dsl::routing_algorithm - .filter( - dsl::created_by - .eq(created_by.to_string()) - .and(dsl::algorithm_for.eq(algorithm_for.to_string())), - ) - .select(dsl::id) - .limit(1) - .get_results_async(&*conn) - .await - } - None => { - dsl::routing_algorithm - .filter(dsl::created_by.eq(created_by.to_string())) - .select(dsl::id) - .limit(1) - .get_results_async(&*conn) - .await - } - }; - - match existing { - Ok(found) => !found.is_empty(), - Err(error) => { - // Reported as "no rules", which routes the caller down the error path it already - // took before this check existed. - logger::error!( - ?error, - created_by = %created_by, - "routing_evaluate: failed to check for existing routing rules" - ); - false - } - } -} - /// Resolves the active routing algorithm for a merchant: Redis cache first, DB with /// cache back-fill on a miss. Extracted so the batch endpoint can resolve once and /// evaluate many parameter sets against the same algorithm. @@ -1022,16 +958,16 @@ pub async fn routing_evaluate( let algorithm = match resolve_active_algorithm(&state, &payload.created_by, algorithm_for).await { Ok(algo) => algo, - // A profile that has rules but none active has deliberately deactivated them, - // and that is an answer rather than a failure: the caller should route by its - // fallback. A profile with no rules at all stays an error, so a caller can still - // tell that the engine has nothing for it. + // No rule to apply is an answer, not a failure: the caller routes by the fallback + // it supplied. This covers a profile whose rules are all deactivated and one that + // has no rules at all — Hyperswitch treats both the same, and a profile awaiting + // migration is reported by the caller, which warns when a cut-over profile gets an + // empty result. Err(e) if matches!( e.get_inner(), EuclidErrors::ActiveRoutingAlgorithmNotFound(_) - ) && profile_has_any_routing_rule(&state, &payload.created_by, algorithm_for) - .await => + ) => { API_REQUEST_COUNTER .with_label_values(&["routing_evaluate", "success"]) @@ -1227,15 +1163,13 @@ pub async fn routing_evaluate_batch( let algorithm = match resolve_active_algorithm(&state, &payload.created_by, algorithm_for).await { Ok(algo) => algo, - // Same semantics as the single endpoint: rules exist but none are active is an - // authoritative "route by your fallback", answered once per entry so eligibility + // Same semantics as the single endpoint, answered once per entry so eligibility // narrowing still runs against each entry's own parameters. Err(e) if matches!( e.get_inner(), EuclidErrors::ActiveRoutingAlgorithmNotFound(_) - ) && profile_has_any_routing_rule(&state, &payload.created_by, algorithm_for) - .await => + ) => { let mut results = Vec::with_capacity(payload.requests.len()); for entry in payload.requests { From 18ef3725a6e83402ad39d92c996b3ebf9091bab8 Mon Sep 17 00:00:00 2001 From: Prajjwal kumar Date: Mon, 31 Aug 2026 13:17:04 +0530 Subject: [PATCH 2/2] fix(routing): require a caller fallback before answering a missing rule The first cut answered every ActiveRoutingAlgorithmNotFound with a 200, which dropped a guard worth keeping: with no rule and no fallback there is nothing to answer with, and no_active_algorithm_response defaults the output to empty, so the caller reads a success that carries nowhere to route. Require a non-empty fallback_output before taking the graceful path. A profile with no rules but a fallback now gets the same answer a deactivated profile already got; one with neither still gets the error it needs. This changes a documented contract: routing-rule-mutations asserted that a merchant with no rules at all errors even when a fallback was supplied. That is precisely the case the shadow traffic fails on, so the test now asserts the fallback answer, and a new case covers the no-fallback guard that the hybrid spec was already enforcing. Co-Authored-By: Claude Opus 5 --- src/euclid/handlers/routing_rules.rs | 17 ++++++++++------- .../api/routing/routing-rule-mutations.spec.ts | 18 +++++++++++++++--- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/src/euclid/handlers/routing_rules.rs b/src/euclid/handlers/routing_rules.rs index e3246fb6..ac8911a3 100644 --- a/src/euclid/handlers/routing_rules.rs +++ b/src/euclid/handlers/routing_rules.rs @@ -958,16 +958,16 @@ pub async fn routing_evaluate( let algorithm = match resolve_active_algorithm(&state, &payload.created_by, algorithm_for).await { Ok(algo) => algo, - // No rule to apply is an answer, not a failure: the caller routes by the fallback - // it supplied. This covers a profile whose rules are all deactivated and one that - // has no rules at all — Hyperswitch treats both the same, and a profile awaiting - // migration is reported by the caller, which warns when a cut-over profile gets an - // empty result. + // A no-rule profile is an answer rather than a failure when the caller supplied a + // fallback to answer with; with nothing to answer with, that stays an error. Err(e) if matches!( e.get_inner(), EuclidErrors::ActiveRoutingAlgorithmNotFound(_) - ) => + ) && payload + .fallback_output + .as_ref() + .is_some_and(|fallback| !fallback.is_empty()) => { API_REQUEST_COUNTER .with_label_values(&["routing_evaluate", "success"]) @@ -1169,7 +1169,10 @@ pub async fn routing_evaluate_batch( if matches!( e.get_inner(), EuclidErrors::ActiveRoutingAlgorithmNotFound(_) - ) => + ) && payload + .fallback_output + .as_ref() + .is_some_and(|fallback| !fallback.is_empty()) => { let mut results = Vec::with_capacity(payload.requests.len()); for entry in payload.requests { diff --git a/tests/api/routing/routing-rule-mutations.spec.ts b/tests/api/routing/routing-rule-mutations.spec.ts index 241e8925..8a796975 100644 --- a/tests/api/routing/routing-rule-mutations.spec.ts +++ b/tests/api/routing/routing-rule-mutations.spec.ts @@ -115,9 +115,9 @@ test.describe('Routing rule mutations (API)', () => { expect(evaluated.body.evaluated_output.map((c: any) => c.gateway_name)).not.toContain('stripe') }) - test('evaluating for a merchant with no rules at all still errors', async ({ api, merchant }) => { - // The opposite state: the engine was never given a rule for this profile, so it has nothing to - // say and the caller should fall back to its own configuration. That stays a 400. + test('evaluating for a merchant with no rules at all answers with the fallback', async ({ api, merchant }) => { + // Indistinguishable to the caller from a profile whose rules are all switched off: both mean + // "route by your fallback", so both are answered the same way. const evaluated = await api.evaluateRoutingAlgorithm( factory.ruleEvaluatePayload(merchant.id, {}, { fallback_output: [factory.gatewayConnector('adyen')], @@ -125,6 +125,18 @@ test.describe('Routing rule mutations (API)', () => { { failOnStatusCode: false }, ) + expect(evaluated.status).toBe(200) + expect(evaluated.body.status).toBe('no_active_algorithm') + expect(evaluated.body.evaluated_output.map((c: any) => c.gateway_name)).toEqual(['adyen']) + }) + + test('evaluating with no rules and no fallback still errors', async ({ api, merchant }) => { + // Nothing to answer with -- a 200 carrying an empty output would read as a decision to nowhere. + const evaluated = await api.evaluateRoutingAlgorithm( + factory.ruleEvaluatePayload(merchant.id, {}, {}), + { failOnStatusCode: false }, + ) + expect(evaluated.status).toBe(400) })