You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "fivetran-ashokborra (via GitHub)" <gi...@apache.org> on 2026/09/24 15:36:50 UTC
[PR] Map MSAL service exception status codes [polaris]
fivetran-ashokborra opened a new pull request, #5605:
URL: https://github.com/apache/polaris/pull/5605
Azure MSAL authentication failures include an HTTP status code in MsalServiceException. The Iceberg exception mapper does not currently recognize this exception type, so these failures can bypass the existing Polaris cloud exception status mapping.
This change adds MSAL4J as a runtime-service compile dependency and maps MsalServiceException using its status code through the existing cloud exception mapping. The existing parameterized mapper test is extended to cover MSAL status codes and the Azure client-secret error message, allowing the IDs and timestamp to vary.
## Checklist
- [ ] ๐ก๏ธ Do not disclose security issues (contact security@apache.org)
- [ ] ๐ Clearly explained why the changes are needed, or linked related issues: Fixes #
- [x] ๐งช Added/updated tests with good coverage, or manually tested (and explained how)
- [ ] ๐ก Added comments for complex logic
- [ ] ๐งพ Updated CHANGELOG.md if needed
- [ ] ๐ Updated documentation in site/content/in-dev/unreleased if needed
Tested:
- ./gradlew format compileAll (passed)
- ./gradlew :polaris-runtime-service:test --tests org.apache.polaris.service.exception.IcebergExceptionMapperTest (passed)
- ./gradlew :polaris-runtime-service:check (still running when PR was opened)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Map MSAL service exception status codes [polaris]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #5605:
URL: https://github.com/apache/polaris/pull/5605#discussion_r4101510554
##########
runtime/service/src/main/java/org/apache/polaris/service/exception/IcebergExceptionMapper.java:
##########
@@ -212,6 +213,7 @@ static int mapExceptionToResponseCode(RuntimeException rex) {
public static int extractHttpCodeFromCloudException(Throwable t) {
return switch (t) {
case S3Exception s3e -> s3e.statusCode();
+ case MsalServiceException mse -> mse.statusCode();
Review Comment:
`MsalServiceException.statusCode()` is nullable (the two-argument constructor used by the new test leaves it unset), but this expression is auto-unboxed to `int` and is later consumed by `mapCloudExceptionToResponseCode`. Any MSAL exception without an HTTP status will therefore throw a `NullPointerException` instead of falling back to the generic 500 mapping. Treat a null status as `UNKNOWN_CLOUD_HTTP_CODE` and add coverage for that constructor path; the current test's override masks it.
##########
runtime/service/src/main/java/org/apache/polaris/service/exception/IcebergExceptionMapper.java:
##########
@@ -212,6 +213,7 @@ static int mapExceptionToResponseCode(RuntimeException rex) {
public static int extractHttpCodeFromCloudException(Throwable t) {
return switch (t) {
case S3Exception s3e -> s3e.statusCode();
+ case MsalServiceException mse -> mse.statusCode();
Review Comment:
This changes the externally visible HTTP status returned for Azure MSAL authentication failures (including the existing 401โ403 and 404โ400 cloud mappings), but no changelog or unreleased user-facing documentation is updated in this change. Please check whether CHANGELOG.md or site/content/in-dev/unreleased needs a note describing the response-mapping change, or document why it is intentionally omitted.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Map MSAL service exception status codes [polaris]
Posted by "fivetran-ashokborra (via GitHub)" <gi...@apache.org>.
fivetran-ashokborra commented on code in PR #5605:
URL: https://github.com/apache/polaris/pull/5605#discussion_r4110694859
##########
runtime/service/src/main/java/org/apache/polaris/service/exception/IcebergExceptionMapper.java:
##########
@@ -212,6 +213,7 @@ static int mapExceptionToResponseCode(RuntimeException rex) {
public static int extractHttpCodeFromCloudException(Throwable t) {
return switch (t) {
case S3Exception s3e -> s3e.statusCode();
+ case MsalServiceException mse -> mse.statusCode();
Review Comment:
I think it makes sense as the status code is nullable
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Map MSAL service exception status codes [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #5605:
URL: https://github.com/apache/polaris/pull/5605#discussion_r4108573642
##########
runtime/service/src/main/java/org/apache/polaris/service/exception/IcebergExceptionMapper.java:
##########
@@ -212,6 +213,7 @@ static int mapExceptionToResponseCode(RuntimeException rex) {
public static int extractHttpCodeFromCloudException(Throwable t) {
return switch (t) {
case S3Exception s3e -> s3e.statusCode();
+ case MsalServiceException mse -> mse.statusCode();
Review Comment:
@fivetran-ashokborra : good point ^ please mention this in CHANGELOG
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Map MSAL service exception status codes [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #5605:
URL: https://github.com/apache/polaris/pull/5605#discussion_r4108577909
##########
runtime/service/src/main/java/org/apache/polaris/service/exception/IcebergExceptionMapper.java:
##########
@@ -212,6 +213,7 @@ static int mapExceptionToResponseCode(RuntimeException rex) {
public static int extractHttpCodeFromCloudException(Throwable t) {
return switch (t) {
case S3Exception s3e -> s3e.statusCode();
+ case MsalServiceException mse -> mse.statusCode();
Review Comment:
@fivetran-ashokborra : WDYT? ^
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Map MSAL service exception status codes [polaris]
Posted by "fivetran-ashokborra (via GitHub)" <gi...@apache.org>.
fivetran-ashokborra commented on code in PR #5605:
URL: https://github.com/apache/polaris/pull/5605#discussion_r4110714119
##########
runtime/service/src/main/java/org/apache/polaris/service/exception/IcebergExceptionMapper.java:
##########
@@ -212,6 +213,7 @@ static int mapExceptionToResponseCode(RuntimeException rex) {
public static int extractHttpCodeFromCloudException(Throwable t) {
return switch (t) {
case S3Exception s3e -> s3e.statusCode();
+ case MsalServiceException mse -> mse.statusCode();
Review Comment:
Added to the changelog
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Map MSAL service exception status codes [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b merged PR #5605:
URL: https://github.com/apache/polaris/pull/5605
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org