Skip to content

Narrow catch clause in createArchive()#561

Open
elharo wants to merge 1 commit into
apache:masterfrom
elharo:fix-catch-exception
Open

Narrow catch clause in createArchive()#561
elharo wants to merge 1 commit into
apache:masterfrom
elharo:fix-catch-exception

Conversation

@elharo

@elharo elharo commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Fixes #559

Replaces the broad catch (Exception e) in createArchive() with catch (MavenArchiverException e), which is the specific checked exception declared by MavenArchiver.createArchive(). The // TODO: improve error handling comment is removed since the exception type is now correctly scoped.

return jarFile;
} catch (Exception e) {
// TODO: improve error handling
} catch (MavenArchiverException e) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ArchiverException needs to be caught too. Just because it can propagate does not mean it should propagate

@elharo
elharo marked this pull request as draft July 19, 2026 16:32
…ArchiverException

Instead of catching the broad Exception type, catch only the specific
exceptions that can actually be thrown: MavenArchiverException (checked
exception from createArchive()) and ArchiverException (RuntimeException
from addDirectory()). The TODO comment is removed since the exception
types are now correctly scoped.
@elharo
elharo force-pushed the fix-catch-exception branch from 897796f to 963d50f Compare July 19, 2026 16:39
@elharo
elharo marked this pull request as ready for review July 19, 2026 16:45
@elharo
elharo requested review from Copilot and cstamas July 19, 2026 16:46

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses issue #559 by narrowing exception handling in AbstractJarMojo#createArchive() so that it no longer catches a generic Exception while assembling the JAR, improving error specificity and avoiding unintended swallowing of unrelated runtime failures.

Changes:

  • Replace catch (Exception e) with a narrower multi-catch for MavenArchiverException and ArchiverException.
  • Remove the // TODO: improve error handling comment now that the catch block is scoped to archiver-related exceptions.
  • Add the required ArchiverException import.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +292 to 293
} catch (MavenArchiverException | ArchiverException e) {
throw new MojoException("Error assembling JAR", e);
@elharo elharo changed the title Narrow catch clause in createArchive() to MavenArchiverException Narrow catch clause in createArchive() Jul 19, 2026
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.

Catch Exception is too broad in createArchive()

2 participants