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

Missing foreign language generates proper Enso error. #3798

Merged
merged 4 commits into from
Oct 17, 2022

Conversation

Akirathan
Copy link
Member

@Akirathan Akirathan commented Oct 14, 2022

Pull Request Description

Trying to invoke a foreign method with non-installed language (either not enabled in the Truffle Context, or not installed in the GraalVM distribution) results in Polyglot_Error, rather than crashing the entire engine.

Important Notes

Checklist

Please include the following checklist in your PR:

  • The documentation has been updated if necessary.
  • All code conforms to the
    Scala,
    Java,
    and
    Rust
    style guides.
  • All code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed: Enso GUI was tested when built using BOTH
      ./run ide build and ./run ide watch.

@Akirathan Akirathan force-pushed the wip/akirathan/engine-crash-183238936 branch from a462842 to 41ded64 Compare October 14, 2022 10:17
@Akirathan Akirathan self-assigned this Oct 14, 2022
RuntimeOptions.LANGUAGE_HOME_OVERRIDE,
Paths.get("../../distribution/component").toFile().getAbsolutePath()
).build();
this.ctx = Context.newBuilder("enso")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried here this.ctx = Context.newBuilder("enso", "js"), but it appears that even with "js" as permitted language, engine does not see "js", even though I have it installed. @hubertp Do you have any suggestions how to include "js" language here? It's not that important, I just wanted to add another test that would successfully call js foreign method, but would fail on python foreign method.

@Akirathan Akirathan force-pushed the wip/akirathan/engine-crash-183238936 branch from 41ded64 to ac9c763 Compare October 14, 2022 10:23
@Akirathan Akirathan marked this pull request as ready for review October 14, 2022 10:23
Copy link
Member

@JaroslavTulach JaroslavTulach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall. Please improve the end-developer message to not mention Truffle, to list all installed languages, etc.

@Akirathan Akirathan added the CI: Ready to merge This PR is eligible for automatic merge label Oct 17, 2022
@mergify mergify bot merged commit 47148a2 into develop Oct 17, 2022
@mergify mergify bot deleted the wip/akirathan/engine-crash-183238936 branch October 17, 2022 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants