Skip to content

Commit af56774

Browse files
kamilkisielakamilkisiela
committed
error handling
1 parent 5336ac7 commit af56774

File tree

4 files changed

+90
-43
lines changed

4 files changed

+90
-43
lines changed

‎bin/router/src/pipeline/error.rs‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,10 @@ use serde::{Deserialize, Serialize};
1717

1818
use crate::{
1919
jwt::errors::JwtForwardingError,
20-
pipeline::header::{RequestAccepts, APPLICATION_GRAPHQL_RESPONSE_JSON_STR},
20+
pipeline::{
21+
header::{RequestAccepts, APPLICATION_GRAPHQL_RESPONSE_JSON_STR},
22+
progressive_override::LabelEvaluationError,
23+
},
2124
};
2225

2326
#[derive(Debug)]
@@ -79,6 +82,8 @@ pub enum PipelineErrorVariant {
7982
PlanExecutionError(PlanExecutionError),
8083
#[error("Failed to produce a plan: {0}")]
8184
PlannerError(Arc<PlannerError>),
85+
#[error(transparent)]
86+
LabelEvaluationError(LabelEvaluationError),
8287

8388
// HTTP Security-related errors
8489
#[error("Required CSRF header(s) not present")]
@@ -95,6 +100,7 @@ impl PipelineErrorVariant {
95100
Self::UnsupportedHttpMethod(_) => "METHOD_NOT_ALLOWED",
96101
Self::PlannerError(_) => "QUERY_PLAN_BUILD_FAILED",
97102
Self::PlanExecutionError(_) => "QUERY_PLAN_EXECUTION_FAILED",
103+
Self::LabelEvaluationError(_) => "OVERRIDE_LABEL_EVALUATION_FAILED",
98104
Self::FailedToParseOperation(_) => "GRAPHQL_PARSE_FAILED",
99105
Self::ValidationErrors(_) => "GRAPHQL_VALIDATION_FAILED",
100106
Self::VariablesCoercionError(_) => "BAD_USER_INPUT",
@@ -122,6 +128,7 @@ impl PipelineErrorVariant {
122128
match (self, prefer_ok) {
123129
(Self::PlannerError(_), _) => StatusCode::INTERNAL_SERVER_ERROR,
124130
(Self::PlanExecutionError(_), _) => StatusCode::INTERNAL_SERVER_ERROR,
131+
(Self::LabelEvaluationError(_), _) => StatusCode::INTERNAL_SERVER_ERROR,
125132
(Self::JwtForwardingError(_), _) => StatusCode::INTERNAL_SERVER_ERROR,
126133
(Self::UnsupportedHttpMethod(_), _) => StatusCode::METHOD_NOT_ALLOWED,
127134
(Self::InvalidHeaderValue(_), _) => StatusCode::BAD_REQUEST,

‎bin/router/src/pipeline/mod.rs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use crate::{
1616
pipeline::{
1717
coerce_variables::coerce_request_variables,
1818
csrf_prevention::perform_csrf_prevention,
19-
error::PipelineError,
19+
error::{PipelineError, PipelineErrorFromAcceptHeader, PipelineErrorVariant},
2020
execution::execute_plan,
2121
execution_request::get_execution_request,
2222
header::{
@@ -143,6 +143,9 @@ pub async fn execute_pipeline(
143143
query: query.to_owned(),
144144
},
145145
}
146+
})
147+
.map_err(|error| {
148+
req.new_pipeline_error(PipelineErrorVariant::LabelEvaluationError(error))
146149
})?;
147150

148151
let query_plan_payload = plan_operation_with_cache(

‎bin/router/src/pipeline/progressive_override.rs‎

Lines changed: 71 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,37 @@ use hive_router_query_planner::{
88
};
99
use rand::Rng;
1010
use vrl::{
11-
compiler::compile as vrl_compile,
12-
compiler::Program as VrlProgram,
13-
compiler::TargetValue as VrlTargetValue,
11+
compiler::{compile as vrl_compile, Program as VrlProgram, TargetValue as VrlTargetValue},
1412
core::Value as VrlValue,
15-
prelude::{state::RuntimeState as VrlState, Context as VrlContext, TimeZone as VrlTimeZone},
13+
prelude::{
14+
state::RuntimeState as VrlState, Context as VrlContext, ExpressionError,
15+
TimeZone as VrlTimeZone,
16+
},
1617
stdlib::all as vrl_build_functions,
1718
value::Secrets as VrlSecrets,
1819
};
1920

20-
use super::error::PipelineError;
21+
#[derive(thiserror::Error, Debug)]
22+
#[error("Failed to compile override label expression for label '{label}': {error}")]
23+
pub struct OverrideLabelsCompileError {
24+
pub label: String,
25+
pub error: String,
26+
}
27+
28+
#[derive(thiserror::Error, Debug)]
29+
pub enum LabelEvaluationError {
30+
#[error(
31+
"Failed to resolve VRL expression for override label '{label}'. Runtime error: {source}"
32+
)]
33+
ExpressionResolutionFailure {
34+
label: String,
35+
source: ExpressionError,
36+
},
37+
#[error(
38+
"VRL expression for override label '{label}' did not evaluate to a boolean. Got: {got}"
39+
)]
40+
ExpressionWrongType { label: String, got: String },
41+
}
2142

2243
/// Contains the request-specific context for progressive overrides.
2344
/// This is stored in the request extensions
@@ -33,24 +54,11 @@ pub struct RequestOverrideContext {
3354
pub fn request_override_context<'req, F>(
3455
override_labels_evaluator: &OverrideLabelsEvaluator,
3556
get_client_request: F,
36-
) -> Result<RequestOverrideContext, PipelineError>
57+
) -> Result<RequestOverrideContext, LabelEvaluationError>
3758
where
3859
F: FnOnce() -> ClientRequestDetails<'req>,
3960
{
40-
// No active flags by default - until we implement it
41-
let active_flags = override_labels_evaluator.evaluate(get_client_request);
42-
43-
// for (flag_name, override_value) in override_labels_config.iter() {
44-
// match override_value {
45-
// LabelOverrideValue::Boolean(true) => {
46-
// active_flags.insert(flag_name.clone());
47-
// }
48-
// // For other cases, we currently do nothing
49-
// _ => {
50-
// // TODO: support expressions
51-
// }
52-
// }
53-
// }
61+
let active_flags = override_labels_evaluator.evaluate(get_client_request)?;
5462

5563
// Generate the random percentage value for this request.
5664
// Percentage is 0 - 100_000_000_000 (100*PERCENTAGE_SCALE_FACTOR)
@@ -116,7 +124,9 @@ pub struct OverrideLabelsEvaluator {
116124
}
117125

118126
impl OverrideLabelsEvaluator {
119-
pub(crate) fn from_config(override_labels_config: &OverrideLabelsConfig) -> Self {
127+
pub(crate) fn from_config(
128+
override_labels_config: &OverrideLabelsConfig,
129+
) -> Result<Self, OverrideLabelsCompileError> {
120130
let mut static_enabled_labels = HashSet::new();
121131
let mut expressions = HashMap::new();
122132
let vrl_functions = vrl_build_functions();
@@ -127,27 +137,41 @@ impl OverrideLabelsEvaluator {
127137
static_enabled_labels.insert(label.clone());
128138
}
129139
LabelOverrideValue::Expression { expression } => {
130-
let compilation_result = vrl_compile(expression, &vrl_functions).unwrap();
140+
let compilation_result =
141+
vrl_compile(expression, &vrl_functions).map_err(|diagnostics| {
142+
OverrideLabelsCompileError {
143+
label: label.clone(),
144+
error: diagnostics
145+
.errors()
146+
.into_iter()
147+
.map(|d| d.code.to_string() + ": " + &d.message)
148+
.collect::<Vec<_>>()
149+
.join(", "),
150+
}
151+
})?;
131152
expressions.insert(label.clone(), compilation_result.program);
132153
}
133154
_ => {} // Skip false booleans
134155
}
135156
}
136157

137-
Self {
158+
Ok(Self {
138159
static_enabled_labels,
139160
expressions,
140-
}
161+
})
141162
}
142163

143-
pub fn evaluate<'req, F>(&self, get_client_request: F) -> HashSet<String>
164+
pub(crate) fn evaluate<'req, F>(
165+
&self,
166+
get_client_request: F,
167+
) -> Result<HashSet<String>, LabelEvaluationError>
144168
where
145169
F: FnOnce() -> ClientRequestDetails<'req>,
146170
{
147171
let mut active_flags = self.static_enabled_labels.clone();
148172

149173
if self.expressions.is_empty() {
150-
return active_flags;
174+
return Ok(active_flags);
151175
}
152176

153177
let client_request = get_client_request();
@@ -165,20 +189,30 @@ impl OverrideLabelsEvaluator {
165189
let mut ctx = VrlContext::new(&mut target, &mut state, &timezone);
166190

167191
for (label, expression) in &self.expressions {
168-
let evaluated_value = expression.resolve(&mut ctx).unwrap();
169-
match evaluated_value {
170-
VrlValue::Boolean(true) => {
171-
active_flags.insert(label.clone());
172-
}
173-
VrlValue::Boolean(false) => {
174-
// Do nothing for false
175-
}
176-
_ => {
177-
// TODO: error handling for non-boolean results
192+
match expression.resolve(&mut ctx) {
193+
Ok(evaluated_value) => match evaluated_value {
194+
VrlValue::Boolean(true) => {
195+
active_flags.insert(label.clone());
196+
}
197+
VrlValue::Boolean(false) => {
198+
// Do nothing for false
199+
}
200+
invalid_value => {
201+
return Err(LabelEvaluationError::ExpressionWrongType {
202+
label: label.clone(),
203+
got: format!("{:?}", invalid_value),
204+
});
205+
}
206+
},
207+
Err(err) => {
208+
return Err(LabelEvaluationError::ExpressionResolutionFailure {
209+
label: label.clone(),
210+
source: err,
211+
});
178212
}
179213
}
180214
}
181215

182-
active_flags
216+
Ok(active_flags)
183217
}
184218
}

‎bin/router/src/shared_state.rs‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use std::sync::Arc;
88

99
use crate::jwt::JwtAuthRuntime;
1010
use crate::pipeline::cors::{CORSConfigError, Cors};
11-
use crate::pipeline::progressive_override::OverrideLabelsEvaluator;
11+
use crate::pipeline::progressive_override::{OverrideLabelsCompileError, OverrideLabelsEvaluator};
1212

1313
pub struct RouterSharedState {
1414
pub validation_plan: ValidationPlan,
@@ -31,9 +31,10 @@ impl RouterSharedState {
3131
parse_cache: moka::future::Cache::new(1000),
3232
cors_runtime: Cors::from_config(&router_config.cors).map_err(Box::new)?,
3333
router_config: router_config.clone(),
34-
override_labels_evaluator: Arc::new(OverrideLabelsEvaluator::from_config(
35-
&router_config.override_labels,
36-
)),
34+
override_labels_evaluator: Arc::new(
35+
OverrideLabelsEvaluator::from_config(&router_config.override_labels)
36+
.map_err(Box::new)?,
37+
),
3738
jwt_auth_runtime,
3839
})
3940
}
@@ -45,4 +46,6 @@ pub enum SharedStateError {
4546
HeaderRuleCompileError(#[from] Box<HeaderRuleCompileError>),
4647
#[error("invalid regex in CORS config: {0}")]
4748
CORSConfigError(#[from] Box<CORSConfigError>),
49+
#[error("invalid override labels config: {0}")]
50+
OverrideLabelsCompileError(#[from] Box<OverrideLabelsCompileError>),
4851
}

0 commit comments

Comments
 (0)