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

More info when critical failure occurs #11092

Merged
merged 12 commits into from
Sep 23, 2024
Merged

More info when critical failure occurs #11092

merged 12 commits into from
Sep 23, 2024

Conversation

hubertp
Copy link
Contributor

@hubertp hubertp commented Sep 16, 2024

Pull Request Description

Log problematic module to help with debugging critical failure.
Right now it's a real guess-work.
Related to #11088.

Log problematic module to help with debugging critical failure.
@hubertp hubertp added the CI: No changelog needed Do not require a changelog entry for this PR. label Sep 16, 2024
@hubertp hubertp mentioned this pull request Sep 17, 2024
@hubertp hubertp requested a review from 4e6 as a code owner September 17, 2024 15:26
@hubertp hubertp requested a review from jdunkerley as a code owner September 18, 2024 14:11
`GeneratedFormatTests.java` not `GeneratedFormatTests..java`
`./run java-gen test --skip-version-check` now works. At least locally.
@JaroslavTulach JaroslavTulach dismissed their stale review September 20, 2024 09:01

Logging is used. Be careful asking for features: Alas now the parser has dependency on slf4j...

@JaroslavTulach
Copy link
Member

There are still some CI failures.

Running java tests requires us knowing all additional dependencies as
they have to be added to the classpath manually. That can only be
ensured by invoking the right sbt target.
build/build/src/rust/parser.rs Outdated Show resolved Hide resolved
build/build/src/rust/parser.rs Outdated Show resolved Hide resolved
@enso-bot
Copy link

enso-bot bot commented Sep 21, 2024

Hubert Plociniczak reports a new STANDUP for the provided date (2024-09-19):

Progress: Struggling with CI rust setup for #11092 (it requires tweaks to java classpath). Still debugging stability issues. Reduced work hours. It should be finished by 2024-09-19.

Next Day: Next day I will be working on the #11092 task. Continue investigating issues.

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.

I'd like to understand what will be the behavior of the newly added error(...) call in various environments Enso code runs in. Thank you.

return Tree.deserialize(message);
} catch (BufferUnderflowException | IllegalArgumentException e) {
LoggerFactory.getLogger(this.getClass())
.error("Unrecoverable parser failure for: {}", input, e);
Copy link
Member

Choose a reason for hiding this comment

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

This code is calling error(Object,Throwable) method. What's its behavior in Enso?

When running runtime-integration-tests

Will it print the message as well as exception to the (CI) console?

When running in enso CLI mode

Will it print the message as well as exception to the (CI) console? Will it do something different?

When running in Enso IDE mode

What will appear in console and what will appear in associated log file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This PR changes nothing when it comes to how log messages are consumed, this is calling a plain slf4j api. The actual printing is dependent on the implementation/configuration, as you know.

If you don't like the format/output of the logger in CLI or tests then please create a ticket for that. IDE works as expected.

Copy link
Member

Choose a reason for hiding this comment

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

If you don't like the format/output of the logger in CLI or tests then please create a ticket for that. IDE works as expected.

Which is?

What will appear in console and what will appear in associated log file?

Will you, please, answer my question?

@hubertp hubertp merged commit 7c41329 into develop Sep 23, 2024
41 checks passed
@hubertp hubertp deleted the wip/hubert/11088-crash branch September 23, 2024 08:59
jdunkerley pushed a commit that referenced this pull request Sep 26, 2024
* More info when critical failure occurs

Log problematic module to help with debugging critical failure.

* One more exception

* s/System.err/Logger.error/

* maybe append slf4j deps

* fix what looks like a long standing typo

`GeneratedFormatTests.java` not `GeneratedFormatTests..java`

* one more typo

* Fix directory where to look for classpath

`./run java-gen test --skip-version-check` now works. At least locally.

* Local is fine, CI is not. More temporary debugging...

* Ensure project's managedClasspath is exported

Running java tests requires us knowing all additional dependencies as
they have to be added to the classpath manually. That can only be
ensured by invoking the right sbt target.

* Move sbt call after graalvm setup

* removing CI debugging

* Apply suggestions from code review

Co-authored-by: Kaz Wesley <[email protected]>

---------

Co-authored-by: Kaz Wesley <[email protected]>
(cherry picked from commit 7c41329)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: No changelog needed Do not require a changelog entry for this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants