-
Notifications
You must be signed in to change notification settings - Fork 334
fix(auth): let ServiceFailureException bubble up for proper HTTP status mapping during auth #2670
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
Conversation
flyrain
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @HotSushi for the change. +1 on more accurate HTTP return code.
runtime/service/src/test/java/org/apache/polaris/service/auth/AuthenticatingAugmentorTest.java
Outdated
Show resolved
Hide resolved
adnanhemani
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
HonahX
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
|
Thanks @HotSushi for the fix! Thanks @singhpk234 @HonahX for the review. |
* Minio testcontainer: allow setting a specific region (apache#2664) ... to remove the need to derive it via the AWS-SDK default mechanism requiring a system property or environment variable. * fix(deps): update dependency org.apache.commons:commons-lang3 to v3.19.0 (apache#2665) * Doc: Add breaking changes section for 1.1 release (apache#2654) * fix(deps): update dependency com.h2database:h2 to v2.4.240 (apache#2668) * fix(auth): let ServiceFailureException bubble up for proper HTTP status mapping during auth (apache#2670) * fix(deps): update immutables to v2.11.4 (apache#2679) * fix(deps): update dependency com.diffplug.spotless:spotless-plugin-gradle to v8 (apache#2669) * chore(deps): update quay.io/keycloak/keycloak docker tag to v26.3.5 (apache#2681) * Last merged commit 2c0bf58 --------- Co-authored-by: Mend Renovate <bot@renovateapp.com> Co-authored-by: Prashant Singh <35593236+singhpk234@users.noreply.github.com> Co-authored-by: Sushant Raikar <sushant.raikar+OSS@snowflake.com>
Summary
Previously, ServiceFailureException from persistence layer timeouts was being
wrapped in AuthenticationFailedException, causing IOException during loadEntity
operations to return 401 Unauthorized instead of the correct 503 Service Unavailable.
This change allows ServiceFailureException to bubble up to IcebergExceptionMapper
which properly maps it to 503 Service Unavailable, while preserving the existing
behavior for other authentication exceptions.
Tests
✅ Added a unit test
✅
./gradlew buildsucceeds