Skip to content

Commit

Permalink
qe: Identify which request in a batch caused error (#3384)
Browse files Browse the repository at this point in the history
* qe: Identify which request in a batch caused error

Adds `batch_request_idx` property to user facing errors. On the client,
that would allow us to build correct error message for `$transaction`
errors.

Ref: prisma/prisma#15433, prisma/prisma#14373
  • Loading branch information
SevInf committed Nov 15, 2022
1 parent 1545b76 commit ea33eec
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 3 deletions.
18 changes: 18 additions & 0 deletions libs/user-facing-errors/src/lib.rs
Expand Up @@ -68,6 +68,9 @@ pub struct Error {
is_panic: bool,
#[serde(flatten)]
inner: ErrorType,

#[serde(skip_serializing_if = "Option::is_none")]
batch_request_idx: Option<usize>,
}

#[derive(Serialize, Deserialize, PartialEq, Debug, Clone)]
Expand All @@ -93,13 +96,18 @@ impl Error {
}
}

pub fn batch_request_idx(&self) -> Option<usize> {
self.batch_request_idx
}

pub fn new_non_panic_with_current_backtrace(message: String) -> Self {
Error {
inner: ErrorType::Unknown(UnknownError {
message,
backtrace: Some(format!("{:?}", backtrace::Backtrace::new())),
}),
is_panic: false,
batch_request_idx: None,
}
}

Expand All @@ -110,6 +118,7 @@ impl Error {
backtrace: None,
}),
is_panic: false,
batch_request_idx: None,
}
}

Expand All @@ -135,6 +144,7 @@ impl Error {
backtrace,
}),
is_panic: true,
batch_request_idx: None,
}
}

Expand All @@ -143,6 +153,7 @@ impl Error {
Error {
inner: ErrorType::Known(err),
is_panic: false,
batch_request_idx: None,
}
}

Expand All @@ -155,6 +166,7 @@ impl Error {
backtrace: None,
}),
is_panic: true,
batch_request_idx: None,
}
}

Expand All @@ -172,6 +184,10 @@ impl Error {
err @ ErrorType::Unknown(_) => panic!("Expected known error, got {:?}", err),
}
}

pub fn set_batch_request_idx(&mut self, batch_request_idx: usize) {
self.batch_request_idx = Some(batch_request_idx)
}
}

pub fn new_backtrace() -> backtrace::Backtrace {
Expand All @@ -183,6 +199,7 @@ impl From<UnknownError> for Error {
Error {
inner: ErrorType::Unknown(unknown_error),
is_panic: false,
batch_request_idx: None,
}
}
}
Expand All @@ -192,6 +209,7 @@ impl From<KnownError> for Error {
Error {
is_panic: false,
inner: ErrorType::Known(known_error),
batch_request_idx: None,
}
}
}
Expand Up @@ -62,6 +62,21 @@ mod transactional {
Ok(())
}

#[connector_test]
async fn batch_request_idx(runner: Runner) -> TestResult<()> {
let queries = vec![
r#"mutation { createOneModelA(data: { id: 1 }) { id }}"#.to_string(),
r#"mutation { createOneModelA(data: { id: 1 }) { id }}"#.to_string(),
];

let batch_results = runner.batch(queries, true, None).await?;
let batch_request_idx = batch_results.errors().get(0).unwrap().batch_request_idx();

assert_eq!(batch_request_idx, Some(1usize));

Ok(())
}

#[connector_test]
async fn one_query(runner: Runner) -> TestResult<()> {
// Existing ModelA in the DB will prevent the nested ModelA creation in the batch.
Expand Down
9 changes: 9 additions & 0 deletions query-engine/core/src/error.rs
Expand Up @@ -70,6 +70,9 @@ pub enum CoreError {

#[error("{}", _0)]
FieldConversionError(#[from] FieldConversionError),

#[error("Error in batch request {request_idx}: {error}")]
BatchError { request_idx: usize, error: Box<CoreError> },
}

impl CoreError {
Expand Down Expand Up @@ -269,6 +272,12 @@ impl From<CoreError> for user_facing_errors::Error {
.into()
}

CoreError::BatchError { request_idx, error } => {
let mut inner_error = user_facing_errors::Error::from(*error);
inner_error.set_batch_request_idx(request_idx);
inner_error
}

_ => user_facing_errors::Error::from_dyn_error(&err),
}
}
Expand Down
15 changes: 12 additions & 3 deletions query-engine/core/src/executor/execute_operation.rs
Expand Up @@ -44,16 +44,25 @@ pub async fn execute_many_operations(

let mut results = Vec::with_capacity(queries.len());

for (query_graph, serializer) in queries {
for (i, (query_graph, serializer)) in queries.into_iter().enumerate() {
increment_counter!(PRISMA_CLIENT_QUERIES_TOTAL);
let operation_timer = Instant::now();
let interpreter = QueryInterpreter::new(conn);
let result = QueryPipeline::new(query_graph, interpreter, serializer)
.execute(trace_id.clone())
.await?;
.await;

histogram!(PRISMA_CLIENT_QUERIES_HISTOGRAM_MS, operation_timer.elapsed());
results.push(Ok(result));

match result {
Ok(result) => results.push(Ok(result)),
Err(error) => {
return Err(crate::CoreError::BatchError {
request_idx: i,
error: Box::new(error),
});
}
}
}

Ok(results)
Expand Down
4 changes: 4 additions & 0 deletions query-engine/request-handlers/src/graphql/response.rs
Expand Up @@ -38,6 +38,10 @@ impl GQLError {
pub fn message(&self) -> &str {
self.user_facing_error.message()
}

pub fn batch_request_idx(&self) -> Option<usize> {
self.user_facing_error.batch_request_idx()
}
}

impl GQLResponse {
Expand Down

0 comments on commit ea33eec

Please sign in to comment.