From 788bc8d021ab0ec2d58837b129c178300b2c2a39 Mon Sep 17 00:00:00 2001 From: Kyle Kelley Date: Mon, 18 Sep 2023 15:07:33 -0700 Subject: [PATCH] fix(cli): Enhanced errors for Jupyter (#20530) --- .../testdata/jupyter/integration_test.ipynb | 167 +++++++++++++----- cli/tools/jupyter/server.rs | 83 +++++++-- 2 files changed, 197 insertions(+), 53 deletions(-) diff --git a/cli/tests/testdata/jupyter/integration_test.ipynb b/cli/tests/testdata/jupyter/integration_test.ipynb index ec6b279735..e9347750d9 100644 --- a/cli/tests/testdata/jupyter/integration_test.ipynb +++ b/cli/tests/testdata/jupyter/integration_test.ipynb @@ -44,7 +44,7 @@ }, { "cell_type": "code", - "execution_count": 1, + "execution_count": 21, "id": "a5d38758", "metadata": { "hidden": true @@ -52,7 +52,7 @@ "outputs": [ { "data": {}, - "execution_count": 1, + "execution_count": 21, "metadata": {}, "output_type": "execute_result" }, @@ -81,7 +81,7 @@ }, { "cell_type": "code", - "execution_count": 2, + "execution_count": 22, "id": "f7fa885a", "metadata": { "hidden": true @@ -89,7 +89,7 @@ "outputs": [ { "data": {}, - "execution_count": 2, + "execution_count": 22, "metadata": {}, "output_type": "execute_result" }, @@ -120,7 +120,7 @@ }, { "cell_type": "code", - "execution_count": 3, + "execution_count": 23, "id": "08a17340", "metadata": { "hidden": true @@ -132,7 +132,7 @@ "{ color: \u001b[32m\"red\"\u001b[39m, area: \u001b[33m10000\u001b[39m }" ] }, - "execution_count": 3, + "execution_count": 23, "metadata": {}, "output_type": "execute_result" } @@ -177,7 +177,7 @@ }, { "cell_type": "code", - "execution_count": 4, + "execution_count": 24, "id": "bbf2c09b", "metadata": { "hidden": true @@ -185,7 +185,7 @@ "outputs": [ { "data": {}, - "execution_count": 4, + "execution_count": 24, "metadata": {}, "output_type": "execute_result" } @@ -207,7 +207,7 @@ }, { "cell_type": "code", - "execution_count": 5, + "execution_count": 25, "id": "d9801d80", "metadata": { "hidden": true @@ -219,7 +219,7 @@ "\u001b[1mnull\u001b[22m" ] }, - "execution_count": 5, + "execution_count": 25, "metadata": {}, "output_type": "execute_result" } @@ -241,7 +241,7 @@ }, { "cell_type": "code", - "execution_count": 6, + "execution_count": 26, "id": "cfaac330", "metadata": { "hidden": true @@ -253,7 +253,7 @@ "\u001b[33mtrue\u001b[39m" ] }, - "execution_count": 6, + "execution_count": 26, "metadata": {}, "output_type": "execute_result" } @@ -275,7 +275,7 @@ }, { "cell_type": "code", - "execution_count": 7, + "execution_count": 27, "id": "ec3be2da", "metadata": { "hidden": true @@ -287,7 +287,7 @@ "\u001b[33m42\u001b[39m" ] }, - "execution_count": 7, + "execution_count": 27, "metadata": {}, "output_type": "execute_result" } @@ -309,7 +309,7 @@ }, { "cell_type": "code", - "execution_count": 8, + "execution_count": 28, "id": "997cf2d7", "metadata": { "hidden": true @@ -321,7 +321,7 @@ "\u001b[32m\"this is a test of the emergency broadcast system\"\u001b[39m" ] }, - "execution_count": 8, + "execution_count": 28, "metadata": {}, "output_type": "execute_result" } @@ -343,7 +343,7 @@ }, { "cell_type": "code", - "execution_count": 9, + "execution_count": 29, "id": "44b63807", "metadata": { "hidden": true @@ -355,7 +355,7 @@ "\u001b[33m31337n\u001b[39m" ] }, - "execution_count": 9, + "execution_count": 29, "metadata": {}, "output_type": "execute_result" } @@ -377,7 +377,7 @@ }, { "cell_type": "code", - "execution_count": 10, + "execution_count": 30, "id": "e10c0d31", "metadata": { "hidden": true @@ -389,7 +389,7 @@ "\u001b[32mSymbol(foo)\u001b[39m" ] }, - "execution_count": 10, + "execution_count": 30, "metadata": {}, "output_type": "execute_result" } @@ -411,7 +411,7 @@ }, { "cell_type": "code", - "execution_count": 11, + "execution_count": 31, "id": "81c99233", "metadata": { "hidden": true @@ -423,7 +423,7 @@ "{ foo: \u001b[32m\"bar\"\u001b[39m }" ] }, - "execution_count": 11, + "execution_count": 31, "metadata": {}, "output_type": "execute_result" } @@ -445,7 +445,7 @@ }, { "cell_type": "code", - "execution_count": 13, + "execution_count": 32, "id": "43c1581b", "metadata": { "hidden": true @@ -457,7 +457,7 @@ "Promise { \u001b[32m\"it worked!\"\u001b[39m }" ] }, - "execution_count": 13, + "execution_count": 32, "metadata": {}, "output_type": "execute_result" } @@ -468,7 +468,7 @@ }, { "cell_type": "code", - "execution_count": 14, + "execution_count": 33, "id": "9a34b725", "metadata": { "hidden": true @@ -483,7 +483,7 @@ "}" ] }, - "execution_count": 14, + "execution_count": 33, "metadata": {}, "output_type": "execute_result" } @@ -494,17 +494,22 @@ }, { "cell_type": "code", - "execution_count": 15, + "execution_count": 34, "id": "b5c7b819", "metadata": { "scrolled": true }, "outputs": [ { - "ename": "Error: this is a test\n at foo (:3:9)\n at :4:3", - "evalue": "", + "ename": "Error", + "evalue": "this is a test", "output_type": "error", - "traceback": [] + "traceback": [ + "Stack trace:", + "Error: this is a test", + " at foo (:3:9)", + " at :4:3" + ] } ], "source": [ @@ -515,7 +520,28 @@ }, { "cell_type": "code", - "execution_count": 16, + "execution_count": 35, + "id": "14844fc9-536e-4121-a9bd-fc2d3f7b6395", + "metadata": {}, + "outputs": [ + { + "ename": "Unknown error", + "evalue": "a party", + "output_type": "error", + "traceback": [ + "Stack trace:", + "\"a party\"", + " at " + ] + } + ], + "source": [ + "throw \"a party\"" + ] + }, + { + "cell_type": "code", + "execution_count": 36, "id": "72d01fdd", "metadata": {}, "outputs": [ @@ -523,13 +549,13 @@ "data": { "text/plain": [ "Promise {\n", - " \u001b[36m\u001b[39m TypeError: Expected string at position 0\n", + " \u001b[36m\u001b[39m TypeError: Expected string at position 1\n", " at Object.readFile (ext:deno_fs/30_fs.js:716:29)\n", " at :2:6\n", "}" ] }, - "execution_count": 16, + "execution_count": 36, "metadata": {}, "output_type": "execute_result" } @@ -540,40 +566,99 @@ }, { "cell_type": "code", - "execution_count": null, + "execution_count": 37, "id": "28cf59d0-6908-4edc-bb10-c325beeee362", "metadata": {}, - "outputs": [], + "outputs": [ + { + "data": {}, + "execution_count": 37, + "metadata": {}, + "output_type": "execute_result" + }, + { + "name": "stdout", + "output_type": "stream", + "text": [ + "Hello from Deno!\n" + ] + } + ], "source": [ "console.log(\"Hello from Deno!\")" ] }, { "cell_type": "code", - "execution_count": null, + "execution_count": 38, "id": "8d5485c3-0da3-43fe-8ef5-a61e672f5e81", "metadata": {}, - "outputs": [], + "outputs": [ + { + "data": {}, + "execution_count": 38, + "metadata": {}, + "output_type": "execute_result" + }, + { + "name": "stdout", + "output_type": "stream", + "text": [ + "\u001b[48;2;21;128;61m\u001b[37m Hello Deno \u001b[0m\n" + ] + } + ], "source": [ "console.log(\"%c Hello Deno \", \"background-color: #15803d; color: white;\");" ] }, { "cell_type": "code", - "execution_count": null, + "execution_count": 39, "id": "1401d9d5-6994-4c7b-b55a-db3c16a1e2dc", "metadata": {}, - "outputs": [], + "outputs": [ + { + "data": { + "text/plain": [ + "\u001b[32m\"Cool 🫡\"\u001b[39m" + ] + }, + "execution_count": 39, + "metadata": {}, + "output_type": "execute_result" + } + ], "source": [ "\"Cool 🫡\"" ] }, { "cell_type": "code", - "execution_count": null, + "execution_count": 40, "id": "7afdaa0a-a2a0-4f52-8c7d-b6c5f237aa0d", "metadata": {}, - "outputs": [], + "outputs": [ + { + "data": {}, + "execution_count": 40, + "metadata": {}, + "output_type": "execute_result" + }, + { + "name": "stdout", + "output_type": "stream", + "text": [ + "┌───────┬────────┐\n", + "│ (idx) │ Values │\n", + "├───────┼────────┤\n", + "│ 0 │ 1 │\n", + "│ 1 │ 2 │\n", + "│ 2 │ 3 │\n", + "└───────┴────────┘\n" + ] + } + ], "source": [ "console.table([1, 2, 3])" ] diff --git a/cli/tools/jupyter/server.rs b/cli/tools/jupyter/server.rs index f0e523e1ea..f53f5209cb 100644 --- a/cli/tools/jupyter/server.rs +++ b/cli/tools/jupyter/server.rs @@ -408,25 +408,84 @@ impl JupyterServer { tokio::time::sleep(std::time::Duration::from_millis(5)).await; } else { let exception_details = exception_details.unwrap(); - let name = if let Some(exception) = exception_details.exception { - if let Some(description) = exception.description { - description - } else if let Some(value) = exception.value { - value.to_string() + + // Determine the exception value and name + let (name, message, stack) = + if let Some(exception) = exception_details.exception { + let result = self + .repl_session + .call_function_on_args( + r#" + function(object) { + if (object instanceof Error) { + const name = "name" in object ? String(object.name) : ""; + const message = "message" in object ? String(object.message) : ""; + const stack = "stack" in object ? String(object.stack) : ""; + return JSON.stringify({ name, message, stack }); + } else { + const message = String(object); + return JSON.stringify({ name: "", message, stack: "" }); + } + } + "# + .into(), + &[exception], + ) + .await?; + + match result.result.value { + Some(serde_json::Value::String(str)) => { + if let Ok(object) = + serde_json::from_str::>(&str) + { + let get = |k| object.get(k).cloned().unwrap_or_default(); + (get("name"), get("message"), get("stack")) + } else { + eprintln!("Unexpected result while parsing JSON {str}"); + ("".into(), "".into(), "".into()) + } + } + _ => { + eprintln!("Unexpected result while parsing exception {result:?}"); + ("".into(), "".into(), "".into()) + } + } } else { - "undefined".to_string() - } + eprintln!("Unexpectedly missing exception {exception_details:?}"); + ("".into(), "".into(), "".into()) + }; + + let stack = if stack.is_empty() { + format!( + "{}\n at ", + serde_json::to_string(&message).unwrap() + ) } else { - "Unknown exception".to_string() + stack + }; + let traceback = format!("Stack trace:\n{stack}") + .split('\n') + .map(|s| s.to_owned()) + .collect::>(); + + let ename = if name.is_empty() { + "Unknown error".into() + } else { + name + }; + + let evalue = if message.is_empty() { + "(none)".into() + } else { + message }; - // TODO(bartlomieju): fill all the fields msg .new_message("error") .with_content(json!({ - "ename": name, - "evalue": " ", // Fake value, otherwise old Jupyter frontends don't show the error - "traceback": [], + "ename": ename, + "evalue": evalue, + "traceback": traceback, })) .send(&mut *self.iopub_socket.lock().await) .await?;