Skip to content
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -483,14 +483,14 @@ private[spark] class ApplicationMaster(
} catch {
case e: InvocationTargetException =>
e.getCause match {
case _: InterruptedException =>
case e: InterruptedException =>
Copy link
Member

Choose a reason for hiding this comment

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

Nit: reusing the name e is ambiguous but also unnecessary; this can be _ too? (Yes the second clause will generate a warning without : Throwable

// Reporter thread can interrupt to stop user class
case e: Exception =>
case _ =>
Copy link
Contributor

Choose a reason for hiding this comment

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

So, the problem is that this was not catching Error, but just Exception, right?

I think this generates a compiler warning, so probably should be case _: Throwable =>. And you can avoid the duplicate getCause calls by giving this variable a name instead of using the placeholder.

finish(FinalApplicationStatus.FAILED,
ApplicationMaster.EXIT_EXCEPTION_USER_CLASS,
"User class threw exception: " + e.getMessage)
"User class threw exception: " + e.getCause.getMessage)
// re-throw to get it logged
throw e
throw e.getCause
}
}
}
Expand Down