Skip to content

Commit

Permalink
refactor: use concrete error type for remaining ops (#26746)
Browse files Browse the repository at this point in the history
  • Loading branch information
crowlKats authored Nov 7, 2024
1 parent db53ec2 commit 1cab4f0
Show file tree
Hide file tree
Showing 11 changed files with 1,424 additions and 567 deletions.
8 changes: 8 additions & 0 deletions cli/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ fn get_resolution_error_class(err: &ResolutionError) -> &'static str {
}
}

fn get_try_from_int_error_class(_: &std::num::TryFromIntError) -> &'static str {
"TypeError"
}

pub fn get_error_class_name(e: &AnyError) -> &'static str {
deno_runtime::errors::get_error_class_name(e)
.or_else(|| {
Expand All @@ -106,5 +110,9 @@ pub fn get_error_class_name(e: &AnyError) -> &'static str {
e.downcast_ref::<ResolutionError>()
.map(get_resolution_error_class)
})
.or_else(|| {
e.downcast_ref::<std::num::TryFromIntError>()
.map(get_try_from_int_error_class)
})
.unwrap_or("Error")
}
4 changes: 2 additions & 2 deletions cli/ops/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ struct PermissionsHolder(Uuid, PermissionsContainer);
pub fn op_pledge_test_permissions(
state: &mut OpState,
#[serde] args: ChildPermissionsArg,
) -> Result<Uuid, AnyError> {
) -> Result<Uuid, deno_runtime::deno_permissions::ChildPermissionError> {
let token = Uuid::new_v4();
let parent_permissions = state.borrow_mut::<PermissionsContainer>();
let worker_permissions = parent_permissions.create_child_permissions(args)?;
Expand Down Expand Up @@ -147,7 +147,7 @@ fn op_dispatch_bench_event(state: &mut OpState, #[serde] event: BenchEvent) {

#[op2(fast)]
#[number]
fn op_bench_now(state: &mut OpState) -> Result<u64, AnyError> {
fn op_bench_now(state: &mut OpState) -> Result<u64, std::num::TryFromIntError> {
let ns = state.borrow::<time::Instant>().elapsed().as_nanos();
let ns_u64 = u64::try_from(ns)?;
Ok(ns_u64)
Expand Down
43 changes: 25 additions & 18 deletions cli/ops/jupyter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ pub fn op_jupyter_input(
state: &mut OpState,
#[string] prompt: String,
is_password: bool,
) -> Result<Option<String>, AnyError> {
) -> Option<String> {
let (last_execution_request, stdin_connection_proxy) = {
(
state.borrow::<Arc<Mutex<Option<JupyterMessage>>>>().clone(),
Expand All @@ -58,11 +58,11 @@ pub fn op_jupyter_input(
if let Some(last_request) = maybe_last_request {
let JupyterMessageContent::ExecuteRequest(msg) = &last_request.content
else {
return Ok(None);
return None;
};

if !msg.allow_stdin {
return Ok(None);
return None;
}

let content = InputRequest {
Expand All @@ -73,7 +73,7 @@ pub fn op_jupyter_input(
let msg = JupyterMessage::new(content, Some(&last_request));

let Ok(()) = stdin_connection_proxy.lock().tx.send(msg) else {
return Ok(None);
return None;
};

// Need to spawn a separate thread here, because `blocking_recv()` can't
Expand All @@ -82,17 +82,25 @@ pub fn op_jupyter_input(
stdin_connection_proxy.lock().rx.blocking_recv()
});
let Ok(Some(response)) = join_handle.join() else {
return Ok(None);
return None;
};

let JupyterMessageContent::InputReply(msg) = response.content else {
return Ok(None);
return None;
};

return Ok(Some(msg.value));
return Some(msg.value);
}

Ok(None)
None
}

#[derive(Debug, thiserror::Error)]
pub enum JupyterBroadcastError {
#[error(transparent)]
SerdeJson(serde_json::Error),
#[error(transparent)]
ZeroMq(AnyError),
}

#[op2(async)]
Expand All @@ -102,7 +110,7 @@ pub async fn op_jupyter_broadcast(
#[serde] content: serde_json::Value,
#[serde] metadata: serde_json::Value,
#[serde] buffers: Vec<deno_core::JsBuffer>,
) -> Result<(), AnyError> {
) -> Result<(), JupyterBroadcastError> {
let (iopub_connection, last_execution_request) = {
let s = state.borrow();

Expand All @@ -125,36 +133,35 @@ pub async fn op_jupyter_broadcast(
content,
err
);
err
JupyterBroadcastError::SerdeJson(err)
})?;

let jupyter_message = JupyterMessage::new(content, Some(&last_request))
.with_metadata(metadata)
.with_buffers(buffers.into_iter().map(|b| b.to_vec().into()).collect());

iopub_connection.lock().send(jupyter_message).await?;
iopub_connection
.lock()
.send(jupyter_message)
.await
.map_err(JupyterBroadcastError::ZeroMq)?;
}

Ok(())
}

#[op2(fast)]
pub fn op_print(
state: &mut OpState,
#[string] msg: &str,
is_err: bool,
) -> Result<(), AnyError> {
pub fn op_print(state: &mut OpState, #[string] msg: &str, is_err: bool) {
let sender = state.borrow_mut::<mpsc::UnboundedSender<StreamContent>>();

if is_err {
if let Err(err) = sender.send(StreamContent::stderr(msg)) {
log::error!("Failed to send stderr message: {}", err);
}
return Ok(());
return;
}

if let Err(err) = sender.send(StreamContent::stdout(msg)) {
log::error!("Failed to send stdout message: {}", err);
}
Ok(())
}
6 changes: 3 additions & 3 deletions cli/ops/testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ struct PermissionsHolder(Uuid, PermissionsContainer);
pub fn op_pledge_test_permissions(
state: &mut OpState,
#[serde] args: ChildPermissionsArg,
) -> Result<Uuid, AnyError> {
) -> Result<Uuid, deno_runtime::deno_permissions::ChildPermissionError> {
let token = Uuid::new_v4();
let parent_permissions = state.borrow_mut::<PermissionsContainer>();
let worker_permissions = parent_permissions.create_child_permissions(args)?;
Expand Down Expand Up @@ -150,7 +150,7 @@ fn op_register_test_step(
#[smi] parent_id: usize,
#[smi] root_id: usize,
#[string] root_name: String,
) -> Result<usize, AnyError> {
) -> usize {
let id = NEXT_ID.fetch_add(1, Ordering::SeqCst);
let origin = state.borrow::<ModuleSpecifier>().to_string();
let description = TestStepDescription {
Expand All @@ -169,7 +169,7 @@ fn op_register_test_step(
};
let sender = state.borrow_mut::<TestEventSender>();
sender.send(TestEvent::StepRegister(description)).ok();
Ok(id)
id
}

#[op2(fast)]
Expand Down
Loading

0 comments on commit 1cab4f0

Please sign in to comment.