Skip to content

Throw proper exception from cached provider#169

Merged
jimmyjames merged 1 commit intomasterfrom
throw-correct-exception
Feb 3, 2023
Merged

Throw proper exception from cached provider#169
jimmyjames merged 1 commit intomasterfrom
throw-correct-exception

Conversation

@jimmyjames
Copy link
Copy Markdown
Contributor

Prior to this change, if using a cached JWK provider (the default), any exception caught when accessing the JWK would be a SigningKeyNotFoundException, with a cause of ExecutionException (thrown by the cache provider).

Since the cause of an exception may be for different reasons (e.g., network exception, KID not found), developers would need to unwrap the SigningKeyNotFoundException to get the ExecutionException, which itself would contain the specific reason for failure (some JwkException).

As raised in #165, this is cumbersome. Instead, this PR throws the causing exception directly, so that developers can more easily identify why the operation failed.

Fixes #165

@jimmyjames jimmyjames requested a review from a team as a code owner February 2, 2023 00:54
return provider.get(keyId);
}
});
return cache.get(cacheKey, () -> provider.get(keyId));
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.

change to lambda syntax, no functional change

@jimmyjames
Copy link
Copy Markdown
Contributor Author

codecov failure due to fall-thru line not being covered. Since ExecutionException is only thrown for checked exceptions, and JwkProvider#get() only throws a JwkException, the instance checking and fallback are a bit overly careful. The effort to test such a case does not seem worthwhile since it's not a realistic scenario and simple mocking will not work as the provider cannot throw anything other than a JwkException.

@jimmyjames jimmyjames merged commit 53264e0 into master Feb 3, 2023
@jimmyjames jimmyjames deleted the throw-correct-exception branch February 3, 2023 02:28
@jimmyjames jimmyjames mentioned this pull request Feb 10, 2023
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.

NetworkException not being thrown when using cached JwkProviderBuilder instead of directly UrlJwkProvider

2 participants