return errors back to user (#4075)

* Log any Errors to stderr

This isn't perfect -- in fact, this is maybe not even very good at all,
but it's better than what we have today.

Currently, when we get an Erorr back from the WebSocket, we drop it in
kcl-lib. The web-app logs these to the console (I can't find my commit
doing that off the top of my head, but I remember doing it) -- so this
is some degree of partity.

This won't be very useful at all for wasm usage, but it will fix issues
with the zoo cli silently breaking with a "WebSocket Closed" error --
which is the same issue I was solving for in the desktop app too.

In the future perhaps this can be a real Error? I'm not totally sure
yet, since we can't align to the request-id, so we can't really tie it
to a specific call (yet).

* add to responses

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu-latest)

* add a test

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* clippy[

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu-latest)

* empty

* fix error

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* updates tests

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* docs

Signed-off-by: Jess Frazelle <github@jessfraz.com>

---------

Signed-off-by: Jess Frazelle <github@jessfraz.com>
Co-authored-by: Jess Frazelle <github@jessfraz.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Jess Frazelle <jessfraz@users.noreply.github.com>
This commit is contained in:
Paul Tagliamonte
2024-10-03 01:05:12 -04:00
committed by GitHub
parent c0afbff377
commit c84c0b0fef
340 changed files with 227 additions and 147 deletions

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

View File

@ -35,6 +35,7 @@ type WebSocketTcpWrite = futures::stream::SplitSink<tokio_tungstenite::WebSocket
pub struct EngineConnection {
engine_req_tx: mpsc::Sender<ToEngineReq>,
responses: Arc<DashMap<uuid::Uuid, WebSocketResponse>>,
pending_errors: Arc<Mutex<Vec<String>>>,
#[allow(dead_code)]
tcp_read_handle: Arc<TcpReadHandle>,
socket_health: Arc<Mutex<SocketHealth>>,
@ -193,6 +194,8 @@ impl EngineConnection {
let responses: Arc<DashMap<uuid::Uuid, WebSocketResponse>> = Arc::new(DashMap::new());
let responses_clone = responses.clone();
let socket_health = Arc::new(Mutex::new(SocketHealth::Active));
let pending_errors = Arc::new(Mutex::new(Vec::new()));
let pending_errors_clone = pending_errors.clone();
let socket_health_tcp_read = socket_health.clone();
let tcp_read_handle = tokio::spawn(async move {
@ -242,6 +245,30 @@ impl EngineConnection {
let mut sd = session_data2.lock().unwrap();
sd.replace(session.clone());
}
WebSocketResponse::Failure(FailureWebSocketResponse {
success: _,
request_id,
errors,
}) => {
if let Some(id) = request_id {
responses_clone.insert(
*id,
WebSocketResponse::Failure(FailureWebSocketResponse {
success: false,
request_id: *request_id,
errors: errors.clone(),
}),
);
} else {
// Add it to our pending errors.
let mut pe = pending_errors_clone.lock().unwrap();
for error in errors {
if !pe.contains(&error.message) {
pe.push(error.message.clone());
}
}
}
}
_ => {}
}
@ -267,6 +294,7 @@ impl EngineConnection {
handle: Arc::new(tcp_read_handle),
}),
responses,
pending_errors,
socket_health,
batch: Arc::new(Mutex::new(Vec::new())),
batch_end: Arc::new(Mutex::new(IndexMap::new())),
@ -351,10 +379,19 @@ impl EngineManager for EngineConnection {
while current_time.elapsed().as_secs() < 60 {
if let Ok(guard) = self.socket_health.lock() {
if *guard == SocketHealth::Inactive {
return Err(KclError::Engine(KclErrorDetails {
message: "Modeling command failed: websocket closed early".to_string(),
source_ranges: vec![source_range],
}));
// Check if we have any pending errors.
let pe = self.pending_errors.lock().unwrap();
if !pe.is_empty() {
return Err(KclError::Engine(KclErrorDetails {
message: pe.join(", ").to_string(),
source_ranges: vec![source_range],
}));
} else {
return Err(KclError::Engine(KclErrorDetails {
message: "Modeling command failed: websocket closed early".to_string(),
source_ranges: vec![source_range],
}));
}
}
}
// We pop off the responses to cleanup our mappings.

View File

@ -15,7 +15,16 @@ pub struct RequestBody {
/// Executes a kcl program and takes a snapshot of the result.
/// This returns the bytes of the snapshot.
pub async fn execute_and_snapshot(code: &str, units: UnitLength) -> anyhow::Result<image::DynamicImage> {
let ctx = new_context(units).await?;
let ctx = new_context(units, true).await?;
do_execute_and_snapshot(&ctx, code).await
}
pub async fn execute_and_snapshot_no_auth(code: &str, units: UnitLength) -> anyhow::Result<image::DynamicImage> {
let ctx = new_context(units, false).await?;
do_execute_and_snapshot(&ctx, code).await
}
async fn do_execute_and_snapshot(ctx: &ExecutorContext, code: &str) -> anyhow::Result<image::DynamicImage> {
let tokens = crate::token::lexer(code)?;
let parser = crate::parser::Parser::new(tokens);
let program = parser.ast()?;
@ -31,7 +40,7 @@ pub async fn execute_and_snapshot(code: &str, units: UnitLength) -> anyhow::Resu
Ok(img)
}
async fn new_context(units: UnitLength) -> anyhow::Result<ExecutorContext> {
async fn new_context(units: UnitLength, with_auth: bool) -> anyhow::Result<ExecutorContext> {
let user_agent = concat!(env!("CARGO_PKG_NAME"), ".rs/", env!("CARGO_PKG_VERSION"),);
let http_client = reqwest::Client::builder()
.user_agent(user_agent)
@ -47,13 +56,19 @@ async fn new_context(units: UnitLength) -> anyhow::Result<ExecutorContext> {
.tcp_keepalive(std::time::Duration::from_secs(600))
.http1_only();
let token = std::env::var("KITTYCAD_API_TOKEN").expect("KITTYCAD_API_TOKEN not set");
let token = if with_auth {
std::env::var("KITTYCAD_API_TOKEN").expect("KITTYCAD_API_TOKEN not set")
} else {
"bad_token".to_string()
};
// Create the client.
let mut client = kittycad::Client::new_from_reqwest(token, http_client, ws_client);
// Set a local engine address if it's set.
if let Ok(addr) = std::env::var("LOCAL_ENGINE_ADDR") {
client.set_base_url(addr);
if with_auth {
client.set_base_url(addr);
}
}
let ctx = ExecutorContext::new(

Binary file not shown.

Before

Width:  |  Height:  |  Size: 66 KiB

After

Width:  |  Height:  |  Size: 66 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 53 KiB

After

Width:  |  Height:  |  Size: 53 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 57 KiB

After

Width:  |  Height:  |  Size: 57 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 57 KiB

After

Width:  |  Height:  |  Size: 57 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 61 KiB

After

Width:  |  Height:  |  Size: 61 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 52 KiB

After

Width:  |  Height:  |  Size: 52 KiB

Some files were not shown because too many files have changed in this diff Show More