Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report Warning in the CLI Execution #9749

Closed
jdunkerley opened this issue Apr 19, 2024 · 16 comments · Fixed by #10842
Closed

Report Warning in the CLI Execution #9749

jdunkerley opened this issue Apr 19, 2024 · 16 comments · Fixed by #10842
Assignees

Comments

@jdunkerley
Copy link
Member

Currently, if you run a script in the CLI any warnings produced will be swallowed silently.

It would be good if any warning that occurs in the main method variables are sent to the logging system so can be seen by the user and captured into Elastic.

@JaroslavTulach
Copy link
Member

With #9773 I can do GraalVM Insight experiment...

Define Insight Script in JavaScript

Create a file enso_insight.js with following content:

let mod = Polyglot.eval("enso", `
from Standard.Base import Warning, Meta

h o = Warning.has_warnings o
w o = Warning.get_all o . at 0 . to_text
t o = o.to_text
`);
let h = mod.eval_expression("h")
let w = mod.eval_expression("w")
let t = mod.eval_expression("t")

function detect_warnings_among_local_variables(ctx, frame) {
  print(`Detecting warnings at the end of ${ctx.name}`);
  for (let n in frame) {
    let o = frame[n];
    if (h(o)) {
      print(`value of ${n} is ${t(o)} has warning ${w(o)}`);
    }
  }
  insight.off('return', detect_warnings_among_local_variables)
}

insight.on('return', detect_warnings_among_local_variables, {
  roots: true,
  rootNameFilter : ".*main"
});

it contains a mixture of Enso and JavaScript code. The JavaScript part "attaches itself" to end of execution of any method with a name that matches .*main regexp. It iterates over all local variables in the frame, checks if they have a Warning attached and if so, it prints it out.

Run any Script with Insight

Create an Enso file with main method. Execute it as:

JAVA_OPTS=-Dpolyglot.insight=enso_insight.js ./built-distribution/enso-engine-*/enso-*/bin/enso  --run Main.enso
Detecting warnings at the end of local.Main::local.Main::main
value of operator32735 is /tmp/temp_file_ensodryrun6824262491422756151.xlsx has warning Warning Only a dry run has occurred, with data written to a temporary file.  Press the Record Once button ❙⬤❙ to write the actual file.

the main method is executed and then all its local variables are scanned to check if they contain a warning. If so, the warning is printed. Similar execution can be used in batch executions.

mergify bot pushed a commit that referenced this issue Apr 24, 2024
While investigating #9749 a JavaScript call to `Polyglot.eval("enso", ....).eval_expression("id")` was made. It crashed as JavaScript isn't using `String` but `TruffleString` to represent strings.
@enso-bot
Copy link

enso-bot bot commented Apr 25, 2024

Jaroslav Tulach reports a new STANDUP for yesterday (2024-04-24):

Progress: - Table.join PR merged: #9410

@enso-bot
Copy link

enso-bot bot commented Apr 26, 2024

Jaroslav Tulach reports a new STANDUP for yesterday (2024-04-25):

Progress: - Float -> Decimal PR is merged: #9740

@enso-bot
Copy link

enso-bot bot commented Apr 27, 2024

Jaroslav Tulach reports a new STANDUP for yesterday (2024-04-26):

Progress: - removed Truffle from runtime-compiler: 3c87142

@enso-bot
Copy link

enso-bot bot commented Apr 30, 2024

Jaroslav Tulach reports a new STANDUP for yesterday (2024-04-29):

Progress: - reopened QA for #9635 (comment)

GitHub
Pull Request Description

part of #7954
Important Notes
The workflow is:

$ npm install -- just in case
$ npm --workspace=enso-gui2 run build-ydoc-server-polyglot -- build the ydocServer.js bundle
...

@enso-bot
Copy link

enso-bot bot commented May 1, 2024

Jaroslav Tulach reports a new STANDUP for yesterday (2024-04-30):

Progress: - presentation for GeeCON: prepare & rehersal

mergify bot pushed a commit that referenced this issue May 2, 2024
As part of changes for #9749, let's rewrite the launcher to Java.
@JaroslavTulach
Copy link
Member

JaroslavTulach commented May 2, 2024

One of the problems of the GraalVM Insight is the need to mix Enso and JavaScript. One way to avoid that is to make sure Warning can be recognized via a polyglot boundary - e.g. it exposes itself via the InteropLibrary in a recognizable way.

let mod = Polyglot.eval("enso", `
from Standard.Base import Warning, Meta

h o = Warning.has_warnings o
w o = Warning.get_all o . at 0 . to_text
t o = o.to_text
`);
let h = mod.eval_expression("h")
let w = mod.eval_expression("w")
let t = mod.eval_expression("t")

That could be done if we pretended that WithWarnings is actually an InteropLibrary.isException...

@JaroslavTulach
Copy link
Member

The AtTheEndOfFirstMethod code gives us alternative way (other than GraalVM Insight) to stop at the end of main method execution.

There already is :list command and it can be used to dump local variables and their warnings.

CCing @radeusgd

@enso-bot enso-bot bot mentioned this issue Aug 14, 2024
2 tasks
@jdunkerley jdunkerley added this to the 2024-08 Release milestone Aug 14, 2024
@jdunkerley
Copy link
Member Author

Can we also return a non-zero exit code if errors occur in main.

@JaroslavTulach
Copy link
Member

The current plan:

The AtTheEndOfFirstMethod code gives us alternative way (other than GraalVM Insight) to stop at the end of main method execution.

There already is :list command and it can be used to dump local variables and their warnings.

  • expand the instrument to do AtTheEndOfFirstMethod code
  • expand the :list command to print warnings and errors
  • by default on and perform :list+warnings+errors-regular (e.g. dump error like values)
  • non-zero when there is any error
  • zero exit when only warnings present

@enso-bot
Copy link

enso-bot bot commented Aug 17, 2024

Jaroslav Tulach reports a new STANDUP for yesterday (2024-08-16):

Progress: - Outdated launcher merged: #10779

@enso-bot
Copy link

enso-bot bot commented Aug 18, 2024

Jaroslav Tulach reports a new STANDUP for yesterday (2024-08-17):

Progress: - 10% speedup: #10837 (comment)

@enso-bot
Copy link

enso-bot bot commented Aug 19, 2024

Jaroslav Tulach reports a new STANDUP for yesterday (2024-08-18):

Progress: - warnings to stderr: 4fda04b

@enso-bot
Copy link

enso-bot bot commented Aug 20, 2024

Jaroslav Tulach reports a new STANDUP for yesterday (2024-08-19):

Progress: - meetings and discussions It should be finished by 2024-08-20.

@enso-bot
Copy link

enso-bot bot commented Aug 21, 2024

Jaroslav Tulach reports a new STANDUP for yesterday (2024-08-20):

Progress: - addressing review comments: #10842

  • meetings and standups It should be finished by 2024-08-20.

@mergify mergify bot closed this as completed in #10842 Aug 21, 2024
@mergify mergify bot closed this as completed in e5f865f Aug 21, 2024
@enso-bot
Copy link

enso-bot bot commented Aug 21, 2024

Jaroslav Tulach reports a new STANDUP for today (2024-08-21):

Progress: - merged: #10842

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 🟢 Accepted
Development

Successfully merging a pull request may close this issue.

2 participants