Skip to content

Commit

Permalink
Allow thrown rhai errors to opt out of logging in the Router
Browse files Browse the repository at this point in the history
  • Loading branch information
@andrewmcgivery
andrewmcgivery committed Oct 4, 2024
1 parent 5a1249c commit a846bfb
Show file tree
Hide file tree
Showing 4 changed files with 129 additions and 20 deletions.
47 changes: 33 additions & 14 deletions apollo-router/src/plugins/rhai/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -333,8 +333,10 @@ macro_rules! gen_map_request {
let result: Result<Dynamic, Box<EvalAltResult>> =
execute(&$rhai_service, &$callback, (shared_request.clone(),));
if let Err(error) = result {
let error_details = process_error(error);
tracing::error!("map_request callback failed: {error_details:#?}");
let error_details = process_error(&error);
if matches!(error_details.log_error, Some(true)) {
tracing::error!("map_request callback failed: {error:#?}");
}
let mut guard = shared_request.lock().unwrap();
let request_opt = guard.take();
return $base::request_failure(request_opt.unwrap().context, error_details);
Expand Down Expand Up @@ -381,8 +383,10 @@ macro_rules! gen_map_router_deferred_request {
let result = execute(&$rhai_service, &$callback, (shared_request.clone(),));

if let Err(error) = result {
tracing::error!("map_request callback failed: {error}");
let error_details = process_error(error);
let error_details = process_error(&error);
if matches!(error_details.log_error, Some(true)){
tracing::error!("map_request callback failed: {error:#?}");
}
let mut guard = shared_request.lock().unwrap();
let request_opt = guard.take();
return $base::request_failure(request_opt.unwrap().context, error_details);
Expand Down Expand Up @@ -468,8 +472,10 @@ macro_rules! gen_map_response {
execute(&$rhai_service, &$callback, (shared_response.clone(),));

if let Err(error) = result {
tracing::error!("map_response callback failed: {error}");
let error_details = process_error(error);
let error_details = process_error(&error);
if matches!(error_details.log_error, Some(true)) {
tracing::error!("map_response callback failed: {error}");
}
let mut guard = shared_response.lock().unwrap();
let response_opt = guard.take();
return $base::response_failure(
Expand Down Expand Up @@ -514,8 +520,10 @@ macro_rules! gen_map_router_deferred_response {
let result =
execute(&$rhai_service, &$callback, (shared_response.clone(),));
if let Err(error) = result {
tracing::error!("map_response callback failed: {error}");
let error_details = process_error(error);
let error_details = process_error(&error);
if matches!(error_details.log_error, Some(true)) {
tracing::error!("map_response callback failed: {error}");
}
let response_opt = shared_response.lock().unwrap().take();
return Ok($base::response_failure(
response_opt.unwrap().context,
Expand Down Expand Up @@ -610,7 +618,8 @@ macro_rules! gen_map_deferred_response {
status: StatusCode::INTERNAL_SERVER_ERROR,
message: Some("rhai execution error: empty response".to_string()),
position: None,
body: None
body: None,
log_error: None
};
return Ok($base::response_failure(
context,
Expand All @@ -631,8 +640,10 @@ macro_rules! gen_map_deferred_response {
let result =
execute(&$rhai_service, &$callback, (shared_response.clone(),));
if let Err(error) = result {
tracing::error!("map_response callback failed: {error}");
let error_details = process_error(error);
let error_details = process_error(&error);
if matches!(error_details.log_error, Some(true)) {
tracing::error!("map_response callback failed: {error}");
}
let mut guard = shared_response.lock().unwrap();
let response_opt = guard.take();
return Ok($base::response_failure(
Expand Down Expand Up @@ -666,8 +677,10 @@ macro_rules! gen_map_deferred_response {
(shared_response.clone(),),
);
if let Err(error) = result {
tracing::error!("map_response callback failed: {error}");
let error_details = process_error(error);
let error_details = process_error(&error);
if matches!(error_details.log_error, Some(true)) {
tracing::error!("map_response callback failed: {error}");
}
let mut guard = shared_response.lock().unwrap();
let response_opt = guard.take();
let $base::DeferredResponse { mut response, .. } = response_opt.unwrap();
Expand Down Expand Up @@ -778,18 +791,20 @@ struct ErrorDetails {
message: Option<String>,
position: Option<Position>,
body: Option<crate::graphql::Response>,
log_error: Option<bool>,
}

fn default_thrown_status_code() -> StatusCode {
StatusCode::INTERNAL_SERVER_ERROR
}

fn process_error(error: Box<EvalAltResult>) -> ErrorDetails {
fn process_error(error: &Box<EvalAltResult>) -> ErrorDetails {
let mut error_details = ErrorDetails {
status: StatusCode::INTERNAL_SERVER_ERROR,
message: Some(format!("rhai execution error: '{error}'")),
position: None,
body: None,
log_error: Some(true),
};

let inner_error = error.unwrap_inner();
Expand All @@ -801,6 +816,10 @@ fn process_error(error: Box<EvalAltResult>) -> ErrorDetails {
} else {
error_details.status = temp_error_details.status;
}

if error_details.log_error.is_none() {
error_details.log_error = Some(true);
}
}
error_details.position = Some(pos.into());
}
Expand Down
54 changes: 48 additions & 6 deletions apollo-router/src/plugins/rhai/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,7 @@ async fn it_can_find_router_global_variables() {
#[tokio::test]
async fn it_can_process_om_subgraph_forbidden() {
if let Err(error) = call_rhai_function("process_subgraph_response_om_forbidden").await {
let processed_error = process_error(error);
let processed_error = process_error(&error);
assert_eq!(processed_error.status, StatusCode::FORBIDDEN);
assert_eq!(
processed_error.message,
Expand All @@ -601,7 +601,7 @@ async fn it_can_process_om_subgraph_forbidden_with_graphql_payload() {
.await
.unwrap_err();

let processed_error = process_error(error);
let processed_error = process_error(&error);
assert_eq!(processed_error.status, StatusCode::FORBIDDEN);
assert_eq!(
processed_error.body,
Expand All @@ -624,7 +624,7 @@ async fn it_can_process_om_subgraph_200_with_graphql_data() {
.await
.unwrap_err();

let processed_error = process_error(error);
let processed_error = process_error(&error);
assert_eq!(processed_error.status, StatusCode::OK);
assert_eq!(
processed_error.body,
Expand All @@ -639,7 +639,7 @@ async fn it_can_process_om_subgraph_200_with_graphql_data() {
#[tokio::test]
async fn it_can_process_string_subgraph_forbidden() {
if let Err(error) = call_rhai_function("process_subgraph_response_string").await {
let processed_error = process_error(error);
let processed_error = process_error(&error);
assert_eq!(processed_error.status, StatusCode::INTERNAL_SERVER_ERROR);
assert_eq!(processed_error.message, Some("rhai execution error: 'Runtime error: I have raised an error (line 223, position 5)'".to_string()));
} else {
Expand All @@ -653,7 +653,7 @@ async fn it_can_process_ok_subgraph_forbidden() {
let error = call_rhai_function("process_subgraph_response_om_ok")
.await
.unwrap_err();
let processed_error = process_error(error);
let processed_error = process_error(&error);
assert_eq!(processed_error.status, StatusCode::OK);
assert_eq!(
processed_error.message,
Expand All @@ -664,7 +664,7 @@ async fn it_can_process_ok_subgraph_forbidden() {
#[tokio::test]
async fn it_cannot_process_om_subgraph_missing_message_and_body() {
if let Err(error) = call_rhai_function("process_subgraph_response_om_missing_message").await {
let processed_error = process_error(error);
let processed_error = process_error(&error);
assert_eq!(processed_error.status, StatusCode::BAD_REQUEST);
assert_eq!(
processed_error.message,
Expand All @@ -679,6 +679,48 @@ async fn it_cannot_process_om_subgraph_missing_message_and_body() {
}
}

#[tokio::test]
async fn it_can_process_subgraph_response_on_log_error_not_defined() {
let error = call_rhai_function("process_subgraph_response_on_log_error_not_defined")
.await
.unwrap_err();
let processed_error = process_error(&error);
assert_eq!(processed_error.status, StatusCode::BAD_REQUEST);
assert_eq!(
processed_error.message,
Some("This is an error to be logged".to_string())
);
assert_eq!(processed_error.log_error, Some(true));
}

#[tokio::test]
async fn it_can_process_subgraph_response_on_log_error_true() {
let error = call_rhai_function("process_subgraph_response_on_log_error_true")
.await
.unwrap_err();
let processed_error = process_error(&error);
assert_eq!(processed_error.status, StatusCode::BAD_REQUEST);
assert_eq!(
processed_error.message,
Some("This is an error to be logged".to_string())
);
assert_eq!(processed_error.log_error, Some(true));
}

#[tokio::test]
async fn it_can_process_subgraph_response_on_log_error_false() {
let error = call_rhai_function("process_subgraph_response_on_log_error_false")
.await
.unwrap_err();
let processed_error = process_error(&error);
assert_eq!(processed_error.status, StatusCode::BAD_REQUEST);
assert_eq!(
processed_error.message,
Some("This is an error not to be logged".to_string())
);
assert_eq!(processed_error.log_error, Some(false));
}

#[tokio::test]
async fn it_mentions_source_when_syntax_error_occurs() {
let err: Box<dyn std::error::Error> = crate::plugin::plugins()
Expand Down
23 changes: 23 additions & 0 deletions apollo-router/tests/fixtures/request_response_test.rhai
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,3 +235,26 @@ fn process_subgraph_response_om_missing_message(response) {
status: 400,
};
}

fn process_subgraph_response_on_log_error_not_defined(response){
throw #{
message: "This is an error to be logged",
status: 400,
};
}

fn process_subgraph_response_on_log_error_true(response){
throw #{
message: "This is an error to be logged",
status: 400,
log_error: true,
};
}

fn process_subgraph_response_on_log_error_false(response){
throw #{
message: "This is an error not to be logged",
status: 400,
log_error: false,
};
}
25 changes: 25 additions & 0 deletions docs/source/customizations/rhai-api.mdx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,31 @@ Rhai throws at the `map_request` layer behave the same as `ControlFlow::Break`,

If the supplied status code is not a valid HTTP status code, then a `500` response code will result.

### Disable logging of thrown errors

By default, the Router will log an error when a Rhai script uses a `throw` to throw an error. You can opt out of this on an individual `throw` by specifying `log_error: false`:

```rhai
fn supergraph_service(service) {
let f = |request| {
throw #{
status: 403,
log_error: false, // Do not log this error
body: #{
errors: [#{
message: `I have raised a 403`,
extensions: #{
code: "ACCESS_DENIED"
}
}]
}
};
};
// Map our request using our closure
service.map_request(f);
}
```

## Timing execution

Your Rhai customization can use the global `Router.APOLLO_START` constant to calculate durations. This is similar to `Epoch` in Unix environments.
Expand Down

0 comments on commit a846bfb

Please sign in to comment.