Skip to content

cleanup runtime exceptions#1299

Merged
elharo merged 3 commits intomasterfrom
i1298
Sep 15, 2025
Merged

cleanup runtime exceptions#1299
elharo merged 3 commits intomasterfrom
i1298

Conversation

@elharo
Copy link
Copy Markdown
Contributor

@elharo elharo commented Aug 12, 2025

This is all test code so it's a safe change as long as test pass

starts on #1298

try (InputStream input = GpgTestUtils.class.getResourceAsStream(pgpKeyResourceName)) {
if (input == null) {
throw new IllegalArgumentException("Secret GPG file not found: " + pgpKeyResourceName);
throw new IOException("Secret GPG file not found: " + pgpKeyResourceName);
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.

The problem here is not the argument. The problem is in the file system external to the program, which suggests a checked exception.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is looked up from the classloader not the filesystem

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.

Nonetheless, it's still external. Unless maybe this is compiled into the plugin's own jar? In which case, IllegalArgumentException is still wrong. Instead it should be something like MissingResourceException

Copy link
Copy Markdown
Member

@kwin kwin Aug 18, 2025

Choose a reason for hiding this comment

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

This is in all cases I can think of a programming mistake therefore unchecked exceptions seems to be fine to me. Which unchecked exception does not really matter to me

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

MissingResourceException is only for ResourceBundles.

@elharo elharo requested a review from kwin August 12, 2025 11:06
@elharo elharo marked this pull request as ready for review August 12, 2025 11:12
@elharo elharo merged commit 903c214 into master Sep 15, 2025
36 checks passed
@elharo elharo deleted the i1298 branch September 15, 2025 17:35
@github-actions
Copy link
Copy Markdown

@elharo Please assign appropriate label to PR according to the type of change.

@github-actions github-actions Bot added this to the 2.2.1 milestone Sep 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants