Implement Runtime.Context.Dataflow_Stack_Trace for dataflow errors thrown from Enso #9625

GregoryTravis · 2024-04-03T20:43:55Z

This is part 1 of #8665, giving the ability to enable full stack traces for dataflow errors that are thrown from Enso code:

        Context.Dataflow_Stack_Trace.with_enabled <|
            throw_something_here

The next part will implement the same check for dataflow errors thrown (returned) directly from Java.

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

The documentation has been updated, if necessary.
Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
All code follows the
Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
All code has been tested:
- Unit tests have been written where possible.
- If GUI codebase was changed, the GUI was tested when built using ./run ide build.

GregoryTravis · 2024-04-03T20:45:04Z

...runtime/src/main/java/org/enso/interpreter/node/expression/builtin/error/ThrowErrorNode.java

-  public Object execute(VirtualFrame giveMeAStackFrame, Object payload) {
-    return DataflowError.withoutTrace(payload, this);
+  public Object execute(VirtualFrame giveMeAStackFrame, State state, Object payload) {
+    return DataflowError.withoutTrace(state, payload, this);


Passing State this way might be a temporary solution; there might be another approach I will use when calling withoutTrace from Java which makes this unnecessary.

How does this State argument change modifies the ThrowErrorNodeGen?

Diff:

112a113,114 > /*** Start of processing argument 0 ***/ > /*** End of processing argument 0 ***/ 127c129 < result = bodyNode.execute(frame, arg1); --- > result = bodyNode.execute(frame, state, arg1); 139c141 < return bodyNode.execute(frame, arg1); --- > return bodyNode.execute(frame, state, arg1);

Yup. So we can get access to State when we have args and we extract it from there. This is tedious, especially when we consider

State and Contexts are not preserved through the Polyglot boundary #7117

It makes more sense to fix #7117 by storing the state in ContextThreadLocal. Then it is going to be easy to access it even when our interpreter code doesn't have access to args.

GregoryTravis · 2024-04-03T20:46:06Z

engine/runtime/src/main/java/org/enso/interpreter/runtime/error/DataflowError.java

@@ -61,6 +55,10 @@ public static DataflowError withoutTrace(Object payload, Node location) {
    }
  }

+  public static DataflowError withoutTrace(Object payload, Node location) {


This overload is here for the calls to withoutTrace which originate in Java and cannot pass a State value. This will be removed in part 2 which implements the same functionality for such calls.

JaroslavTulach · 2024-04-04T13:19:15Z

distribution/lib/Standard/Base/0.0.0-dev/src/Runtime.enso

@@ -161,6 +164,7 @@ type Context
        case self of
            Input  -> "Input"
            Output -> "Output"
+            Dataflow_Stack_Trace -> "Dataflow_Stack_Trace "


Why is there a space at the end of the text literal?

JaroslavTulach · 2024-04-04T13:22:02Z

test/Base_Tests/src/Runtime/Stack_Traces_Spec.enso

+        shallow_stack_trace.length . should_equal 1
+        shallow_stack_trace.at 0 . name . should_equal (names.at 0)
+
+        Context.Dataflow_Stack_Trace.with_enabled <|


Yup, this is the right test.

JaroslavTulach · 2024-04-04T13:24:28Z

...runtime/src/main/java/org/enso/interpreter/node/expression/builtin/error/ThrowErrorNode.java

-  public Object execute(VirtualFrame giveMeAStackFrame, Object payload) {
-    return DataflowError.withoutTrace(payload, this);
+  public Object execute(VirtualFrame giveMeAStackFrame, State state, Object payload) {
+    return DataflowError.withoutTrace(state, payload, this);


How does this State argument change modifies the ThrowErrorNodeGen?

GregoryTravis · 2024-04-04T20:27:07Z

As I discussed with @JaroslavTulach, it makes sense to merge this PR to implement the context control for dataflow errors coming from Enso code, and do errors coming from Java in another PR.

radeusgd · 2024-04-05T15:43:47Z

distribution/lib/Standard/Base/0.0.0-dev/src/Runtime.enso

@@ -150,6 +150,9 @@ type Context
    ## PRIVATE
       ADVANCED
    Output
+    ## PRIVATE


Please add a newline between the constructors.

Suggested change

## PRIVATE

## PRIVATE

engine/runtime/src/main/java/org/enso/interpreter/runtime/error/DataflowError.java

radeusgd

Looks great

radeusgd · 2024-04-05T15:47:48Z

engine/runtime/src/main/java/org/enso/interpreter/runtime/error/DataflowError.java

-  public static DataflowError withoutTrace(Object payload, Node location) {
+  public static DataflowError withoutTrace(State state, Object payload, Node location) {


Should we rename this constructor?

It is not really withoutTrace, more like withDefaultTrace or something like that.

I'd also like to update the comment to mention that it has the default location (top elem) or full stack depending on the Context.

…g/enso into wip/gmt/8665-stack-trace-context

JaroslavTulach · 2024-07-30T12:44:27Z

engine/runtime/src/main/java/org/enso/interpreter/runtime/error/DataflowError.java

    assert payload != null;
-    if (assertsOn) {
+    boolean attachFullStackTrace =


There is a regression in standard library benchmarks

stdlib benchmarks regression: DataflowError with stack #10702

My hypothesis: It has to be caused the call to hasContextEnabled check on this line.

GregoryTravis added 2 commits April 3, 2024 14:40

works in standalone

d6e7954

enso throw test

f470b02

GregoryTravis changed the title ~~Wip/gmt/8665 stack trace context~~ Implement Runtime.Context.Dataflow_Stack_Trace for dataflow errors thrown from Enso Apr 3, 2024

GregoryTravis commented Apr 3, 2024

View reviewed changes

GregoryTravis marked this pull request as ready for review April 4, 2024 13:11

GregoryTravis requested review from jdunkerley, radeusgd, AdRiley, 4e6, JaroslavTulach, hubertp and Akirathan as code owners April 4, 2024 13:11

JaroslavTulach reviewed Apr 4, 2024

View reviewed changes

GregoryTravis added 3 commits April 4, 2024 11:02

Merge branch 'develop' into wip/gmt/8665-stack-trace-context

19ebc50

Merge branch 'develop' into wip/gmt/8665-stack-trace-context

03fb4b0

review

7240326

GregoryTravis requested a review from JaroslavTulach April 4, 2024 20:27

fmt

43ac9df

GregoryTravis added the CI: No changelog needed Do not require a changelog entry for this PR. label Apr 4, 2024

enso-bot bot mentioned this pull request Apr 5, 2024

Autoscoped constructors in a vector #9630

Merged

2 tasks

JaroslavTulach approved these changes Apr 5, 2024

View reviewed changes

Merge branch 'develop' into wip/gmt/8665-stack-trace-context

a20a3df

radeusgd reviewed Apr 5, 2024

View reviewed changes

engine/runtime/src/main/java/org/enso/interpreter/runtime/error/DataflowError.java Show resolved Hide resolved

radeusgd approved these changes Apr 5, 2024

View reviewed changes

radeusgd reviewed Apr 5, 2024

View reviewed changes

add missing type

0988f8b

enso-bot bot mentioned this pull request Apr 6, 2024

Autoscoping for Table aggregate #9614

Closed

Merge branch 'develop' into wip/gmt/8665-stack-trace-context

85cbdef

Merge branch 'wip/gmt/8665-stack-trace-context' of github.com:enso-or…

f5b6b1e

…g/enso into wip/gmt/8665-stack-trace-context

Akirathan approved these changes Apr 15, 2024

View reviewed changes

GregoryTravis added 4 commits April 15, 2024 13:31

Merge branch 'develop' into wip/gmt/8665-stack-trace-context

93e1949

rename withoutTrace

85e328c

make tests pass

4e73998

Merge branch 'develop' into wip/gmt/8665-stack-trace-context

5f32f3f

JaroslavTulach mentioned this pull request Apr 26, 2024

[WIP] Add logging for UnresolvedMethod bug #9763

Closed

5 tasks

merge

3cbb0e2

GregoryTravis requested a review from marthasharkey as a code owner July 26, 2024 16:05

GregoryTravis added the CI: Ready to merge This PR is eligible for automatic merge label Jul 26, 2024

mergify bot merged commit f0e9616 into develop Jul 26, 2024
42 checks passed

mergify bot deleted the wip/gmt/8665-stack-trace-context branch July 26, 2024 19:20

Akirathan mentioned this pull request Jul 29, 2024

stdlib benchmarks regression: DataflowError with stack #10702

Closed

GregoryTravis mentioned this pull request Jul 29, 2024

Implement Runtime.Context.Dataflow_Stack_Trace for dataflow errors thrown from Java #10712

Open

enso-bot bot mentioned this pull request Jul 29, 2024

Migrate WithWarnings to use EnsoHashMap to speed them up significantly #8682

Closed

JaroslavTulach reviewed Jul 30, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Runtime.Context.Dataflow_Stack_Trace for dataflow errors thrown from Enso #9625

Implement Runtime.Context.Dataflow_Stack_Trace for dataflow errors thrown from Enso #9625

GregoryTravis commented Apr 3, 2024

GregoryTravis Apr 3, 2024

JaroslavTulach Apr 4, 2024

GregoryTravis Apr 4, 2024

JaroslavTulach Apr 5, 2024

GregoryTravis Apr 3, 2024

JaroslavTulach Apr 4, 2024

GregoryTravis Apr 4, 2024

JaroslavTulach Apr 4, 2024

JaroslavTulach Apr 4, 2024

GregoryTravis commented Apr 4, 2024

radeusgd Apr 5, 2024

radeusgd left a comment

radeusgd Apr 5, 2024

GregoryTravis Apr 15, 2024

JaroslavTulach Jul 30, 2024

		public static DataflowError withoutTrace(Object payload, Node location) {
		public static DataflowError withoutTrace(State state, Object payload, Node location) {

Implement Runtime.Context.Dataflow_Stack_Trace for dataflow errors thrown from Enso #9625

Implement Runtime.Context.Dataflow_Stack_Trace for dataflow errors thrown from Enso #9625

Conversation

GregoryTravis commented Apr 3, 2024

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GregoryTravis commented Apr 4, 2024

Choose a reason for hiding this comment

radeusgd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment