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

Better handling of uncaught exceptions in main and user threads #3423

Merged
merged 8 commits into from Aug 9, 2023

Conversation

WojciechMazur
Copy link
Contributor

Fixes #3418

@LeeTibbert
Copy link
Contributor

Suggested change in title: s/uncought/uncaught/

Copy link
Member

@tanishiking tanishiking left a comment

Choose a reason for hiding this comment

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

It looks good to me, now we will have

Exception in thread "main"
Exception: java.lang.StringIndexOutOfBoundsException thrown from the UncaughtExceptionHandler in thread "main"
... (stacktrace)

when exception is thrown while rendering another exception 👍

I left one question about setDefaultUncaughtExceptionHandler.


// ScalaNativeSpecific exception handler, used when executing user-provied handlers, in main
// Duplicate in NativeThread.threadEntryPoint
private[java] final def uncaughtExceptionInternal(
Copy link
Member

Choose a reason for hiding this comment

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

[note] We come here when scala-native app fails with uncaught exception

Comment on lines 363 to 364
try uncaughtException(t, e)
catch {
Copy link
Member

Choose a reason for hiding this comment

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

[note]
here we catch the exception thrown while rendering another exception

def getDefaultUncaughtExceptionHandler(): UncaughtExceptionHandler =
defaultExceptionHandler
def setDefaultUncaughtHandler(eh: UncaughtExceptionHandler): Unit =
def setDefaultUncaughtExceptionHandler(eh: UncaughtExceptionHandler): Unit =
Copy link
Member

Choose a reason for hiding this comment

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

Maybe a silly question, who calls setDefaultUncaughtExceptionHandler? I couldn't find the caller of this method in this project, and ThreadGroup#uncaughtException always goes this branch

case null =>
val threadName = s""""${thread.getName()}""""
System.err.print(s"Exception in thread $threadName")
throwable.printStackTrace(System.err)
?

Copy link
Member

Choose a reason for hiding this comment

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

Nevermind, it's gonna be used by users like Thread.setDefaultUncaughtExceptionHandler(new MyExceptionHandler());

Copy link
Member

@tanishiking tanishiking left a comment

Choose a reason for hiding this comment

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

Review changes approve -> request changes, just because CI is failing :)

@JD557
Copy link
Contributor

JD557 commented Aug 9, 2023

Suggested change in title: s/uncought/uncaught/

Not just in the title, some places in the code also mention uncought

@WojciechMazur WojciechMazur changed the title Better handling of uncought exceptions in main and user threads Better handling of uncaught exceptions in main and user threads Aug 9, 2023
@WojciechMazur WojciechMazur merged commit e479a1e into scala-native:main Aug 9, 2023
79 checks passed
@WojciechMazur WojciechMazur deleted the runtime/main-ueh branch August 9, 2023 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can we provide a friendlier default handler for unhandled exceptions?
4 participants