Skip to content

Commit

Permalink
node-drivers: unwinding panics into napi errors, that can be converti…
Browse files Browse the repository at this point in the history
…ble to quaint errors (#4106)

* Wrap panics in a quaint::ErrorKind::RawConnectorError

* chore: streamline nested blocks

---------

Co-authored-by: jkomyno <skiabo97@gmail.com>
  • Loading branch information
Miguel Fernández and jkomyno committed Aug 4, 2023
1 parent 8f598f6 commit b93b473
Show file tree
Hide file tree
Showing 9 changed files with 123 additions and 39 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions quaint/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,11 @@ impl Error {
pub fn is_closed(&self) -> bool {
matches!(self.kind, ErrorKind::ConnectionClosed)
}

// Builds an error from a raw error coming from the connector
pub fn raw_connector_error(status: String, reason: String) -> Error {
Error::builder(ErrorKind::RawConnectorError { status, reason }).build()
}
}

impl fmt::Display for Error {
Expand All @@ -143,6 +148,9 @@ impl fmt::Display for Error {

#[derive(Debug, Error)]
pub enum ErrorKind {
#[error("Error in the underlying connector ({}): {}", status, reason)]
RawConnectorError { status: String, reason: String },

#[error("Error querying the database: {}", _0)]
QueryError(Box<dyn std::error::Error + Send + Sync + 'static>),

Expand Down
4 changes: 4 additions & 0 deletions query-engine/connectors/sql-query-connector/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,10 @@ impl From<prisma_models::ConversionFailure> for SqlError {
impl From<quaint::error::Error> for SqlError {
fn from(e: quaint::error::Error) -> Self {
match QuaintKind::from(e) {
QuaintKind::RawConnectorError { status, reason } => Self::RawError {
code: status,
message: reason,
},
QuaintKind::QueryError(qe) => Self::QueryError(qe),
QuaintKind::QueryInvalidInput(qe) => Self::QueryInvalidInput(qe),
e @ QuaintKind::IoError(_) => Self::ConnectionError(e),
Expand Down
1 change: 1 addition & 0 deletions query-engine/js-connectors/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ tracing-core = "0.1"
num-bigint = "0.4.3"
bigdecimal = "0.3.0"
chrono = "0.4.20"
futures = "0.3"

napi.workspace = true
napi-derive.workspace = true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ model Child {
parentId String? @unique
non_unique String?
id String @id
@@unique([c_1, c_2])
}

Expand All @@ -89,5 +90,6 @@ model Parent {
p_2 String
non_unique String?
id String @id
@@unique([p_1, p_2])
}
47 changes: 47 additions & 0 deletions query-engine/js-connectors/src/error.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
use futures::{Future, FutureExt};
use napi::Error as NapiError;
use quaint::error::Error as QuaintError;
use std::{any::Any, panic::AssertUnwindSafe};

/// transforms a napi error into a quaint error copying the status and reason
/// properties over
pub(crate) fn into_quaint_error(napi_err: NapiError) -> QuaintError {
let status = napi_err.status.as_ref().to_owned();
let reason = napi_err.reason.clone();

QuaintError::raw_connector_error(status, reason)
}

/// catches a panic thrown during the executuin of an asynchronous closure and transforms it into
/// the Error variant of a napi::Result.
pub(crate) async fn async_unwinding_panic<F, R>(fut: F) -> napi::Result<R>
where
F: Future<Output = napi::Result<R>>,
{
AssertUnwindSafe(fut)
.catch_unwind()
.await
.unwrap_or_else(panic_to_napi_err)
}

/// catches a panic thrown during the execution of a closure and transforms it into a the Error
/// variant of a napi::Result.
pub(crate) fn unwinding_panic<F, R>(f: F) -> napi::Result<R>
where
F: Fn() -> napi::Result<R>,
{
match std::panic::catch_unwind(std::panic::AssertUnwindSafe(f)) {
Ok(result) => result,
Err(panic_payload) => panic_to_napi_err(panic_payload),
}
}

fn panic_to_napi_err<R>(panic_payload: Box<dyn Any + Send>) -> napi::Result<R> {
panic_payload
.downcast_ref::<&str>()
.map(|s| -> String { (*s).to_owned() })
.or_else(|| panic_payload.downcast_ref::<String>().map(|s| s.to_owned()))
.map(|message| Err(napi::Error::from_reason(format!("PANIC: {message}"))))
.ok_or(napi::Error::from_reason("PANIC: unknown panic".to_string()))
.unwrap()
}
1 change: 1 addition & 0 deletions query-engine/js-connectors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
//! plus some transformation of types to adhere to what a quaint::Value expresses.
//!

mod error;
mod proxy;
mod queryable;
pub use queryable::JsQueryable;
85 changes: 49 additions & 36 deletions query-engine/js-connectors/src/proxy.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
use core::panic;
use std::str::FromStr;
use std::sync::{Arc, Condvar, Mutex};

use napi::bindgen_prelude::{FromNapiValue, Promise as JsPromise, ToNapiValue};
use napi::threadsafe_function::{ErrorStrategy, ThreadsafeFunction};
use napi::{JsObject, JsString};
use napi_derive::napi;

use crate::error::*;
use psl::JsConnectorFlavor;
use quaint::connector::ResultSet as QuaintResultSet;
use quaint::Value as QuaintValue;
use std::str::FromStr;

// TODO(jkomyno): import these 3rd-party crates from the `quaint-core` crate.
use bigdecimal::BigDecimal;
Expand Down Expand Up @@ -349,55 +351,66 @@ impl From<FlavoredJSResultSet> for QuaintResultSet {

impl Proxy {
pub async fn query_raw(&self, params: Query) -> napi::Result<JSResultSet> {
let promise = self.query_raw.call_async::<JsPromise<JSResultSet>>(params).await?;
let value = promise.await?;
Ok(value)
async_unwinding_panic(async {
let promise = self.query_raw.call_async::<JsPromise<JSResultSet>>(params).await?;
let value = promise.await?;
Ok(value)
})
.await
}

pub async fn execute_raw(&self, params: Query) -> napi::Result<u32> {
let promise = self.execute_raw.call_async::<JsPromise<u32>>(params).await?;
let value = promise.await?;
Ok(value)
async_unwinding_panic(async {
let promise = self.execute_raw.call_async::<JsPromise<u32>>(params).await?;
let value = promise.await?;
Ok(value)
})
.await
}

pub async fn version(&self) -> napi::Result<Option<String>> {
let version = self.version.call_async::<Option<String>>(()).await?;
Ok(version)
async_unwinding_panic(async {
let version = self.version.call_async::<Option<String>>(()).await?;
Ok(version)
})
.await
}

pub async fn close(&self) -> napi::Result<()> {
self.close.call_async::<()>(()).await
async_unwinding_panic(async { self.close.call_async::<()>(()).await }).await
}

pub fn is_healthy(&self) -> napi::Result<bool> {
let result_arc = Arc::new((Mutex::new(None), Condvar::new()));
let result_arc_clone: Arc<(Mutex<Option<bool>>, Condvar)> = result_arc.clone();

let set_value_callback = move |value: bool| {
let (lock, cvar) = &*result_arc_clone;
unwinding_panic(|| {
let result_arc = Arc::new((Mutex::new(None), Condvar::new()));
let result_arc_clone: Arc<(Mutex<Option<bool>>, Condvar)> = result_arc.clone();

let set_value_callback = move |value: bool| {
let (lock, cvar) = &*result_arc_clone;
let mut result_guard = lock.lock().unwrap();
*result_guard = Some(value);
cvar.notify_one();

Ok(())
};

// Should anyone find a less mind-boggling way to retrieve the result of a synchronous JS
// function, please do so.
self.is_healthy.call_with_return_value(
(),
napi::threadsafe_function::ThreadsafeFunctionCallMode::Blocking,
set_value_callback,
);

// wait for `set_value_callback` to be called and to set the result
let (lock, cvar) = &*result_arc;
let mut result_guard = lock.lock().unwrap();
*result_guard = Some(value);
cvar.notify_one();

Ok(())
};

// Should anyone find a less mind-boggling way to retrieve the result of a synchronous JS
// function, please do so.
self.is_healthy.call_with_return_value(
(),
napi::threadsafe_function::ThreadsafeFunctionCallMode::Blocking,
set_value_callback,
);

// wait for `set_value_callback` to be called and to set the result
let (lock, cvar) = &*result_arc;
let mut result_guard = lock.lock().unwrap();
while result_guard.is_none() {
result_guard = cvar.wait(result_guard).unwrap();
}
while result_guard.is_none() {
result_guard = cvar.wait(result_guard).unwrap();
}

Ok(result_guard.unwrap_or_default())
Ok(result_guard.unwrap_or_default())
})
}
}

Expand Down
13 changes: 10 additions & 3 deletions query-engine/js-connectors/src/queryable.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use crate::proxy::{self, FlavoredJSResultSet, JSResultSet, Proxy, Query};
use crate::{
error::into_quaint_error,
proxy::{self, FlavoredJSResultSet, JSResultSet, Proxy, Query},
};
use async_trait::async_trait;
use napi::JsObject;
use psl::JsConnectorFlavor;
Expand Down Expand Up @@ -153,9 +156,13 @@ impl JsQueryable {
let serialization_span = info_span!("js:query:args", user_facing = true, "length" = %len);
let query = Self::build_query(sql, params).instrument(serialization_span).await;

// Todo: convert napi::Error to quaint::error::Error.
let sql_span = info_span!("js:query:sql", user_facing = true, "db.statement" = %sql);
let result_set = self.proxy.query_raw(query).instrument(sql_span).await.unwrap();
let result_set = self
.proxy
.query_raw(query)
.instrument(sql_span)
.await
.map_err(into_quaint_error)?;

let len = result_set.len();
let deserialization_span = info_span!("js:query:result", user_facing = true, "length" = %len);
Expand Down

0 comments on commit b93b473

Please sign in to comment.