Include original error location in Panic.rethrow
#11893
Assignees
Labels
Comments
|
With following code from Standard.Base import all
deep_error n:Integer = if n <= 0 then Error.throw "Here is an error" else
deep_error n-1
deep_throw n:Integer ~action = if n <= 0 then action else
deep_throw n-1 action
main =
action n = Runtime.Context.Dataflow_Stack_Trace.with_enabled <|
deep_error n
deep_throw 10 (Panic.rethrow (action 10))I seem to receive following stacktrace: Such a stacktrace contains five useless initial lines and misses the ones from
Yes, turning |
|
I get following stacktrace for the above program: with these changes: enso$ git diff --staged
diff --git distribution/lib/Standard/Base/0.0.0-dev/src/Panic.enso distribution/lib/Standard/Base/0.0.0-dev/src/Panic.enso
index a69f0522d3..8e7c7420c4 100644
--- distribution/lib/Standard/Base/0.0.0-dev/src/Panic.enso
+++ distribution/lib/Standard/Base/0.0.0-dev/src/Panic.enso
@@ -87,7 +87,7 @@ type Panic
example_rethrow = Panic.rethrow Examples.throw_error
rethrow : (Any ! Any) -> Any
- rethrow value = value.catch Any Panic.throw
+ rethrow value = panic_rethrow value
## ADVANCED
ICON panic
@@ -202,7 +202,7 @@ type Panic
> Example
Rethrow an error as a panic from the middle of a block, and handle it
- with an explicilt handler.
+ with an explicit handler.
handler e =
IO.println 'Caught: '+e.to_text
@@ -296,3 +296,5 @@ type Wrapped_Dataflow_Error
## PRIVATE
to_display_text self -> Text = "Wrapped_Dataflow_Error: "+self.payload.to_display_text
+
+panic_rethrow err = @Builtin_Method "Panic.rethrow"
\ No newline at end of file
diff --git engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/error/RethrowPanicNode.java engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/error/RethrowPanicNode.java
new file mode 100644
index 0000000000..682d69a4a6
--- /dev/null
+++ engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/error/RethrowPanicNode.java
@@ -0,0 +1,29 @@
+package org.enso.interpreter.node.expression.builtin.error;
+
+import com.oracle.truffle.api.dsl.Specialization;
+import com.oracle.truffle.api.frame.VirtualFrame;
+import com.oracle.truffle.api.nodes.Node;
+import org.enso.interpreter.dsl.AcceptsError;
+import org.enso.interpreter.dsl.BuiltinMethod;
+import org.enso.interpreter.runtime.error.DataflowError;
+import org.enso.interpreter.runtime.state.State;
+
+@BuiltinMethod(type = "Panic", name = "rethrow", description = "Turns error into Panic.")
+public abstract class RethrowPanicNode extends Node {
+
+ RethrowPanicNode() {}
+
+ static RethrowPanicNode build() {
+ return RethrowPanicNodeGen.create();
+ }
+
+ abstract Object execute(VirtualFrame frame, State state, @AcceptsError Object obj);
+
+ @Specialization
+ Object doExecute(VirtualFrame frame, State state, Object obj) {
+ if (obj instanceof DataflowError err) {
+ throw err.rethrow();
+ }
+ return obj;
+ }
+}
diff --git engine/runtime/src/main/java/org/enso/interpreter/runtime/error/DataflowError.java engine/runtime/src/main/java/org/enso/interpreter/runtime/error/DataflowError.java
index a4b7fa2d2a..3889e26f2c 100644
--- engine/runtime/src/main/java/org/enso/interpreter/runtime/error/DataflowError.java
+++ engine/runtime/src/main/java/org/enso/interpreter/runtime/error/DataflowError.java
@@ -212,4 +212,12 @@ public final class DataflowError extends AbstractTruffleException {
Type getType(@Bind("$node") Node node) {
return EnsoContext.get(node).getBuiltins().dataflowError();
}
+
+ public PanicException rethrow() throws PanicException {
+ if (getStackTraceElementLimit() == 1) {
+ throw new PanicException(getPayload(), this, getLocation());
+ } else {
+ throw new PanicException(this);
+ }
+ }
}
diff --git engine/runtime/src/main/java/org/enso/interpreter/runtime/error/PanicException.java engine/runtime/src/main/java/org/enso/interpreter/runtime/error/PanicException.java
index e1a5a55ae4..e4f9e6e5ae 100644
--- engine/runtime/src/main/java/org/enso/interpreter/runtime/error/PanicException.java
+++ engine/runtime/src/main/java/org/enso/interpreter/runtime/error/PanicException.java
@@ -59,6 +59,12 @@ public final class PanicException extends AbstractTruffleException {
this.payload = payload;
}
+ /** package private for use from {@link DataflowError#rethrow}. */
+ PanicException(DataflowError err) {
+ super(err);
+ this.payload = err.getPayload();
+ }
+
/**
* Returns the payload in the panic.
* |
|
If I modify the above program to just: from Standard.Base import all
deep_error n:Integer = if n <= 0 then Error.throw "Here is an error" else
deep_error n-1
deep_throw n:Integer ~action = if n <= 0 then action else
deep_throw n-1 action
main =
action n = # Runtime.Context.Dataflow_Stack_Trace.with_enabled <|
deep_error n
deep_throw 10 (Panic.rethrow (action 10))then the stack trace will contain single location of the Shall I create a PR, @radeusgd? |
Shouldn't the stack trace be after your patch? |
|
I think ideally we'd somehow keep both the original location of the rethrown dataflow error and the stack trace where it was rethrown - this way we retain all information about the 'path' the error took. But if keeping both is hard, I think being able to see the original location in rethrow is already an improvement. But do you think we could maybe see both somehow? E.g. by still keeping the rethrow location in the new panic payload, but also including the original error location as a 'cause' parameter? |
Looks like we already do see them both - concatenated. This is a modified version of the sample: from Standard.Base import all
deep_error n:Integer = if n <= 0 then Error.throw "Here is an error" else
deep_error n-1
deep_throw n:Integer ~action = if n <= 0 then action else
deep_throw n-1 action
main =
action n = Runtime.Context.Dataflow_Stack_Trace.with_enabled <|
deep_error n
deep_throw 10 (Panic.rethrow (action 10))and looks like the Truffle exception system really concatenates the stack of the error with the stack of the panic: This works almost automatically. Customization maybe be harder. |
|
Hmm interesting. The concatenation is:
Ideally it would be great if we could indicate this 'border' e.g. by adding some fake border delimiter frame or in any other way. But if it's too complicated I guess this is good enough. |
|
It is all quite closed system. There is no write access to
No way to mangle with the stacktrace meanwhile. However, at the end, when we are asked to provide the final stacktrace we could do some filtering, concatenation, etc. Also there is a way to chain cause of exceptions - e.g. the original |
Yes that is what I was trying to suggest. I think that would be the ideal solution.
Yeah, unfortunately a few places would need to be updated to take this into account, that is true. Still I think that would be the best solution as it will give us detailed and at the same time, legible stack traces. And the amount of changes shouldn't be too great, so IMHO it could be worth it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Currently,
Panic.rethrowcatches the error, converts it to a raw value (discarding stack trace info) and rethrows as a panic. The Panic's stack trace points to the place where the error was rethrown. All information on where the error originated from is lost at this point.To aid with debugging, it would be good to be able to still get the original location of the error.
Still, the
rethrowlocation may too be useful (to see where the error re-surfaced and became a panic). So ideally we should retain both. Perhaps we could add acausefield to the panic, that could hold the original dataflow error, and ensure that when printing the stack trace we first print the frames of the rethrow location, followed by aCaused bysection that prints the frames/location of the original dataflow error?Likely to do this we may have to turn
Panic.rethrowinto a builtin and extend thePanicException.The text was updated successfully, but these errors were encountered: