Throwing an unhandled exception in Polyglot Java results in a null in the UI and no caught error message

[wdanilo]

This task is automatically imported from the old Task Issue Board and it was originally created by James Dunkerley.
Original issue is here.

---

Throwing a NPE within Java caused an error when command line:

Execution finished with an error: null
        at <java> org.enso.table.data.index.MultiValueIndex.makeCrossTabTable(MultiValueIndex.java:185)
        at <enso> case_branch(C:\Repos\Enso\ide\built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\lib\Standard\Table\0.0.0-dev\src\Data\Table.enso:1299:21-122)

Within the UI get no error message and the viz shows null. Error from Language Server shown below.

Comments:

(James Dunkerley - Dec 20, 2022)

---

``` from Standard.Base import Panic polyglot java import java.lang.NullPointerException

main =
x = NullPointerException.new
Panic.throw x

Easily reproducible via cli

built-distribution/enso-engine-0.0.0-dev-linux-amd64/enso-0.0.0-dev/bin/enso --no-ir-caches --run ~/work/repos/enso/test-cases/thrownpe.enso
Execution finished with an error: null
at Panic.throw(Internal)
at thrownpe.main(thrownpe.enso:6:5-17)

It's because NPEs message is null. (Hubert Plociniczak - Dec 20, 2022)
<hr />

[JaroslavTulach]

What exactly is the bug report? I see it is concerned with NullPointerException. However as of Feb 17, 2023 I see following behavior.

Visualizing NullPointerException

I can throw, Panic.catch and visualize NullPointerException - there are no null messages anymore anywhere:

When running the same program from CLI I get:

java.lang.NullPointerException

which is correct in my opinion - again there is no null anywhere.

Panic with NullPointerException

Modifying the program to propagate the panic (e.g. main = thr) leads to following behavior in CLI:

$ enso --run .
Execution finished with an error: null
        at <enso> Panic.throw(Internal)
        at <enso> Main.thr(/home/devel/enso/projects/Unnamed/src/Main.enso:12:5-17)
        at <enso> Main.main(/home/devel/enso/projects/Unnamed/src/Main.enso:7:8-10)

e.g. yes, there is a null instead of NullPointerException. Doing the same in the IDE leads to an empty visualization:

and following message being printed in project-manager console:

[warn] [enso] Execution of function main failed (class com.oracle.truffle.host.HostException).

[JaroslavTulach]

With the #5684 fixes I see following message being delivered to the IDE. Regardless of that, there is no error indication:

I believe the real fix needs to be done on the IDE side. Somehow handle the executionContext/executionStatus with kind: "Error". Possibly displaying the message, if it is there (it will there for sure once #5684 gets integrated). I am asking for a new triage.

[vitvakatu]

It seems IDE ignores executionStatus updates completely. @farmaazon please correct me if I'm wrong.

To display errors, we use information provided by ExpressionUpdate message. Why in this case the engine does not use it? What is the difference between an error provided by ExpressionUpdate and an error from executionStatus, @JaroslavTulach ?

[JaroslavTulach]

The world of Enso errors is rich (compilation errors, values with warnings, errors and panics). The message above is result of a Panic.throw. I have no idea why it send the message it sends.

[farmaazon]

It seems IDE ignores executionStatus updates completely. @farmaazon please correct me if I'm wrong.

Yes, that's true. The reason is (not sure if still valid) that the diagnostics are text-oriented, giving us line number instead of node id.

[JaroslavTulach]

#5684 only makes sure the executionContext/executionStatus message is delivered. Now it is up to the IDE to tell the user (a message in status bar?) that such problem happened.

[farmaazon]

#5684 only makes sure the executionContext/executionStatus message is delivered. Now it is up to the IDE to tell the user (a message in status bar?) that such problem happened.

Of course, we can display the message in status bar somewhere, but it will be still inferior UX from just having a concrete node colored. Is the latter impossible to implement in the engine?

[farmaazon]

I don't think this should be assigned to me. As I wrote, the panics should be reported in values update payload, so we can display them on nodes. Displaying global execution problems deserves its own task and will be done after we implement messages for the user. However, it does not fix the problem reported by this issue: reporting things in executionStatus should not be a normal routine.

@JaroslavTulach Please answer my question: isn't fixing the problem correctly (by sending Panic payload) impossible in the engine?

Cc @wdanilo @xvcgreg

[wdanilo]

CC @sylwiabr as she took over planning from Greg

[JaroslavTulach]

Of course, we can display the message in status bar somewhere,

That would, in my opinion, be a great start, @farmaazon. Just use the information you have - executionContext/executionStatus. It's likely an hour long fix, right? If so, I'd start with that, fix the current state (when the information is completely missing) and then ...

but it will be still inferior UX from just having a concrete node colored. Is the latter impossible to implement in the engine?

... I would report a feature request to improve the UX by changing the protocol API.

... (by sending Panic payload) ...

In any case, I'd like you to show/propose/demonstate the requeted API changes by modifying language server specification. I am not completely sure what "by sending Panic payload" means in terms of language server protocol...

[farmaazon]

That would, in my opinion, be a great start, @farmaazon. Just use the information you have - executionContext/executionStatus. It's likely an hour long fix, right? If so, I'd start with that, fix the current state (when the information is completely missing) and then ...

Not so fast, we don't have a way to display long messages to the user yet. It's part of #5201 - definitely not an hour of work.

In any case, I'd like you to show/propose/demonstate the requeted API changes by modifying language server specification. I am not completely sure what "by sending Panic payload" means in terms of language server protocol...

I don't need to change API, because we already have an API for reporting panics. Citing the protocol specification:

type ExpressionUpdatePayload = Value | DatafalowError | Panic | Pending;

/* ... */

/**
 * Indicates that the expression failed with the runtime exception.
 */
interface Panic {
  /**
   * The error message.
   */
  message: String;

  /**
   * The stack trace.
   */
  trace: ExpressionId[];
}

If you don't send Panic payload when a runtime error occurs, then you don't comply to the API. If you send null or empty string in message string, then you also don't comply to the API, which clearly states that this should be an error message.

[JaroslavTulach]

Not so fast, we don't have a way to display long messages to the user yet. It's part of #5201 - definitely not an hour of work.

We don't need a full blown solution right now:

• Don't let the IDE throw away executionContext/executionStatus messages #5734

would be enough to fix the most painful problem

I don't need to change API, because we already have an API for reporting panics. Citing ...

OK. Thank you for the explanation and showing me the missing piece of the puzzle.

[farmaazon]

We don't need a full blown solution right now:

This partial solution has serious drawbacks, I described in the PR. But even with the perfect implementation with all diagnostics displayed in a neat multiline notification will be inferior to have this exception reported as Panic, with nice node coloring and proper propagation through the graph (so the user will quickly find it if it's in a nested function).

I've checked what we receive in IDE. First, I created such a graph, where a panic is indeed properly reported, even with the polyglot type (the message, however, is a bit of obscure for user not accustomed to java):

The issue appears when I want to throw an exception instance:

The only expressionUpdate message I receive is:

{"jsonrpc":"2.0","method":"executionContext/expressionUpdates","params":{"contextId":"8ddbdd9b-c01b-4084-94b3-df9d66f82d55","updates":[{"expressionId":"4c563f3b-0bb1-4524-947d-12c68cb6010d","type":null,"methodPointer":null,"profilingInfo":[],"fromCache":true,"payload":{"type":"Pending","message":null,"progress":null}},{"expressionId":"7e5e1bce-8a85-4e3e-91a8-69064a78e806","type":null,"methodPointer":null,"profilingInfo":[],"fromCache":true,"payload":{"type":"Pending","message":null,"progress":null}},{"expressionId":"19d1c03f-4799-4acf-9108-33d8024aa4ec","type":null,"methodPointer":null,"profilingInfo":[],"fromCache":true,"payload":{"type":"Pending","message":null,"progress":null}},{"expressionId":"a40fb8ad-11e3-45c2-940e-ad1331b3665a","type":null,"methodPointer":null,"profilingInfo":[],"fromCache":true,"payload":{"type":"Pending","message":null,"progress":null}},{"expressionId":"007ea7f6-1739-4ba6-bd05-97baeabca0ed","type":null,"methodPointer":null,"profilingInfo":[],"fromCache":true,"payload":{"type":"Pending","message":null,"progress":null}},{"expressionId":"c8261625-f4cb-4afc-9b0d-99fddc7198a6","type":null,"methodPointer":null,"profilingInfo":[],"fromCache":true,"payload":{"type":"Pending","message":null,"progress":null}},{"expressionId":"238845ae-e890-4f4b-8b6b-53d29142cd1d","type":null,"methodPointer":null,"profilingInfo":[],"fromCache":true,"payload":{"type":"Pending","message":null,"progress":null}},{"expressionId":"2ca22482-e25c-4e4b-a080-46744c848208","type":null,"methodPointer":null,"profilingInfo":[],"fromCache":true,"payload":{"type":"Pending","message":null,"progress":null}},{"expressionId":"8dd29e9a-a5fc-407d-9275-c95b0853ebe8","type":null,"methodPointer":null,"profilingInfo":[],"fromCache":true,"payload":{"type":"Pending","message":null,"progress":null}},{"expressionId":"29e20c13-7e9a-4a09-92e9-03484de53bbc","type":null,"methodPointer":null,"profilingInfo":[],"fromCache":true,"payload":{"type":"Pending","message":null,"progress":null}},{"expressionId":"ce8aa384-d43c-4853-a234-a6a993938d6d","type":null,"methodPointer":null,"profilingInfo":[],"fromCache":true,"payload":{"type":"Pending","message":null,"progress":null}},{"expressionId":"19d1c03f-4799-4acf-9108-33d8024aa4ec","type":null,"methodPointer":null,"profilingInfo":[],"fromCache":true,"payload":{"type":"Pending","message":null,"progress":null}},{"expressionId":"0818b28f-46a9-4d9c-9eea-cc0d175d36fc","type":null,"methodPointer":null,"profilingInfo":[],"fromCache":true,"payload":{"type":"Pending","message":null,"progress":null}},{"expressionId":"129b1d1c-3e27-43ac-a0e4-32dbb428be59","type":null,"methodPointer":null,"profilingInfo":[],"fromCache":true,"payload":{"type":"Pending","message":null,"progress":null}},{"expressionId":"bcd222bd-1194-484b-b2e3-0558feec8dbb","type":null,"methodPointer":null,"profilingInfo":[],"fromCache":true,"payload":{"type":"Pending","message":null,"progress":null}},{"expressionId":"7e5e1bce-8a85-4e3e-91a8-69064a78e806","type":null,"methodPointer":null,"profilingInfo":[],"fromCache":true,"payload":{"type":"Pending","message":null,"progress":null}},{"expressionId":"8dd29e9a-a5fc-407d-9275-c95b0853ebe8","type":"Standard.Builtins.Main.Unresolved_Symbol","methodPointer":null,"profilingInfo":[{"ExecutionTime":{"nanoTime":3280}}],"fromCache":false,"payload":{"type":"Value","warnings":null}},{"expressionId":"0818b28f-46a9-4d9c-9eea-cc0d175d36fc","type":null,"methodPointer":null,"profilingInfo":[{"ExecutionTime":{"nanoTime":2559}}],"fromCache":false,"payload":{"type":"Value","warnings":null}},{"expressionId":"ce8aa384-d43c-4853-a234-a6a993938d6d","type":null,"methodPointer":null,"profilingInfo":[{"ExecutionTime":{"nanoTime":1196376}}],"fromCache":false,"payload":{"type":"Value","warnings":null}},{"expressionId":"29e20c13-7e9a-4a09-92e9-03484de53bbc","type":"Standard.Base.Nothing.Nothing","methodPointer":null,"profilingInfo":[{"ExecutionTime":{"nanoTime":1656200}}],"fromCache":false,"payload":{"type":"Value","warnings":null}},{"expressionId":"238845ae-e890-4f4b-8b6b-53d29142cd1d","type":"Standard.Builtins.Main.Unresolved_Symbol","methodPointer":null,"profilingInfo":[{"ExecutionTime":{"nanoTime":2564}}],"fromCache":false,"payload":{"type":"Value","warnings":null}},{"expressionId":"c8261625-f4cb-4afc-9b0d-99fddc7198a6","type":"Standard.Base.Panic.Panic.type","methodPointer":null,"profilingInfo":[{"ExecutionTime":{"nanoTime":1929}}],"fromCache":false,"payload":{"type":"Value","warnings":null}},{"expressionId":"bcd222bd-1194-484b-b2e3-0558feec8dbb","type":null,"methodPointer":null,"profilingInfo":[{"ExecutionTime":{"nanoTime":41173}}],"fromCache":false,"payload":{"type":"Value","warnings":null}}]}}

As you see, no panic payload is reported, only panic type. Moreover, there are "pending" expressions which seem to be never computed (I did not receive any further expressionUpdate).

So, the proper solution for this issue is fixing Panic sending (those never ending pending expressions also deserves fixing).

[enso-bot]

Jaroslav Tulach reports a new STANDUP for yesterday (2023-03-06):

Progress: - Report AbstractTruffleException as a Panic: merged #5794

• Review I/O contexts: https://github.com/enso-org/design/pull/53/files#r1126574783
• Review of Review File/Data read and read_text warnings #5799
• Review: Pre compute suggestion db during build time #5698 (review)
• Investing frgaal compiler error: https://github.com/enso-org/enso/pull/5698/files/335305a4cc52c6569b38168945e4b9fd77387206..b4a98a4c475aca85d4692c76eddcaa91313a3f08#r1125872971
• Reported problem: Avoid using Javac internals in Enso DSL #5817
• measuring collapse node performance
• Discussing DATE_TIME_FORMATTER sharing between libs & builtin with James
• Discussing https://discord.com/channels/@me/1082263497016090634/1082274926976847884
• Meetings & Demo meetings
• Helping with regexp - probably related to: Equality with polyglot numbers is broken #5177
• fix for GregT: commit a197a74 Coerce polyglot values to supported Enso types It should be finished by 2023-03-07.

Next Day: #5177 + Performance & stability