You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "oguzhanunlu (via GitHub)" <gi...@apache.org> on 2026/05/21 08:37:43 UTC
[PR] API, Core: Add exceptions for OAuth2 token endpoint errors [iceberg]
oguzhanunlu opened a new pull request, #16507:
URL: https://github.com/apache/iceberg/pull/16507
**Summary**
Follow-up to #15746 / #16059 . Same unreliable triage pattern, at the OAuth2 token endpoint instead of `/v1/config`.
**Background**
OAuth2 token endpoint failures (RFC 6749 §5.2) currently surface as generic `BadRequestException` / `NotAuthorizedException`, with the error type stringified into the exception message. Consumers that triage on the error type must regex-parse the message to recover it.
This PR introduces exceptions that carry the OAuth2 error type as a field accessible via a marker interface:
```
OAuth2Error (marker, String errorType())
OAuth2BadRequestException extends BadRequestException implements OAuth2Error (400)
OAuth2NotAuthorizedException extends NotAuthorizedException implements OAuth2Error (401)
```
`OAuthErrorHandler.accept` now throws these for all six RFC 6749 §5.2 codes (invalid_request, invalid_client, invalid_grant, unauthorized_client, unsupported_grant_type, invalid_scope).
**Design**
Two classes (per HTTP status), not six (per error type). Different reasons within an HTTP status class are unified under one exception per status, consistent with how `BadRequestException` covers all 400s currently.
Backward compatible: `getMessage()` output is byte-identical and existing catch blocks on `BadRequestException` / `NotAuthorizedException` continue to fire.
Adds six unit tests that pin which exception is thrown for each OAuth2 error type.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core: Add exceptions for OAuth2 token endpoint errors [iceberg]
Posted by "oguzhanunlu (via GitHub)" <gi...@apache.org>.
oguzhanunlu commented on PR #16507:
URL: https://github.com/apache/iceberg/pull/16507#issuecomment-4968066887
Hi @adutra , is there anything I can do to move this forward? Thanks!
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core: Add exceptions for OAuth2 token endpoint errors [iceberg]
Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on PR #16507:
URL: https://github.com/apache/iceberg/pull/16507#issuecomment-4980675956
> Thanks for the 2nd approval @adutra ! I was wondering if we could merge the PR, given your approval.
Unfortunately I'm not a committer on this project. Let's see if @nastra could review your PR.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core: Add exceptions for OAuth2 token endpoint errors [iceberg]
Posted by "oguzhanunlu (via GitHub)" <gi...@apache.org>.
oguzhanunlu commented on PR #16507:
URL: https://github.com/apache/iceberg/pull/16507#issuecomment-5766951620
Thanks @kevinjqliu ! I applied [the diff](https://github.com/apache/iceberg/commit/6d47849bc269964d52638deb08972af6a89bb959.diff) for the small nit in https://github.com/apache/iceberg/pull/16507/changes/a56d59fd1ee666737f4219b013b8929779e5046e.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core: Add exceptions for OAuth2 token endpoint errors [iceberg]
Posted by "nssalian (via GitHub)" <gi...@apache.org>.
nssalian commented on PR #16507:
URL: https://github.com/apache/iceberg/pull/16507#issuecomment-5486653902
@nastra @singhpk234 @stevenzwu would you have bandwidth to look at this? This was asked to be added to the 1.12 by @adutra so would be great to get it reviewed and see if it can be merged.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core: Add exceptions for OAuth2 token endpoint errors [iceberg]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on PR #16507:
URL: https://github.com/apache/iceberg/pull/16507#issuecomment-5766314096
taking a look at this, sorry this lingered for a long time @oguzhanunl. Appreciate the pr!
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core: Add exceptions for OAuth2 token endpoint errors [iceberg]
Posted by "oguzhanunlu (via GitHub)" <gi...@apache.org>.
oguzhanunlu commented on PR #16507:
URL: https://github.com/apache/iceberg/pull/16507#issuecomment-4874454350
Thanks for the feedback @adutra ! I dropped both in the new commit.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core: Add exceptions for OAuth2 token endpoint errors [iceberg]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on PR #16507:
URL: https://github.com/apache/iceberg/pull/16507#issuecomment-5767303393
thanks! lets give others some time to take a look 😄
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core: Add exceptions for OAuth2 token endpoint errors [iceberg]
Posted by "oguzhanunlu (via GitHub)" <gi...@apache.org>.
oguzhanunlu commented on PR #16507:
URL: https://github.com/apache/iceberg/pull/16507#issuecomment-4692808939
Just a friendly ping for review when you have time @kevinjqliu . Happy to address any feedback. Thanks in advance!
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core: Add exceptions for OAuth2 token endpoint errors [iceberg]
Posted by "oguzhanunlu (via GitHub)" <gi...@apache.org>.
oguzhanunlu commented on PR #16507:
URL: https://github.com/apache/iceberg/pull/16507#issuecomment-4980241059
Thanks for the 2nd approval @adutra ! I was wondering if we could merge the PR, given your approval.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core: Add exceptions for OAuth2 token endpoint errors [iceberg]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on PR #16507:
URL: https://github.com/apache/iceberg/pull/16507#issuecomment-5957651193
Would appreciate a couple more eyes on the public API aspect of this change before merging.
@rdblue @stevenzwu @danielcweeks would you have a chance to take a look? The main question for me is whether exposing the OAuth error type through `OAuth2Error` + the two existing HTTP exception subclasses is the right API shape, or whether we're adding too much OAuth-specific API surface.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core: Add exceptions for OAuth2 token endpoint errors [iceberg]
Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 commented on code in PR #16507:
URL: https://github.com/apache/iceberg/pull/16507#discussion_r3929625410
##########
api/src/main/java/org/apache/iceberg/exceptions/OAuth2BadRequestException.java:
##########
@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.exceptions;
+
+import com.google.errorprone.annotations.FormatMethod;
+import com.google.errorprone.annotations.FormatString;
+
+/**
+ * Bad-request exception raised when an OAuth2 token-endpoint response carries one of the {@code
+ * 400}-class error codes from RFC 6749 §5.2 (e.g. {@code invalid_request}, {@code invalid_grant},
+ * {@code unauthorized_client}, {@code unsupported_grant_type}, {@code invalid_scope}).
+ */
+public class OAuth2BadRequestException extends BadRequestException implements OAuth2Error {
Review Comment:
while i understand this would be ideal, there is no such exception defined in the spec
https://github.com/apache/iceberg/blob/6dec25e430b33a8b4f623b14940110d459581826/open-api/rest-catalog-open-api.yaml#L4804
would like to hear from others on what they think considering that
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core: Add exceptions for OAuth2 token endpoint errors [iceberg]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on PR #16507:
URL: https://github.com/apache/iceberg/pull/16507#issuecomment-5766697942
@danielcweeks do you have any thoughts on this change?
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core: Add exceptions for OAuth2 token endpoint errors [iceberg]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #16507:
URL: https://github.com/apache/iceberg/pull/16507#discussion_r4065757120
##########
api/src/main/java/org/apache/iceberg/exceptions/OAuth2BadRequestException.java:
##########
@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.exceptions;
+
+import com.google.errorprone.annotations.FormatMethod;
+import com.google.errorprone.annotations.FormatString;
+
+/**
+ * Bad-request exception raised when an OAuth2 token-endpoint response carries one of the {@code
+ * 400}-class error codes from RFC 6749 §5.2 (e.g. {@code invalid_request}, {@code invalid_grant},
+ * {@code unauthorized_client}, {@code unsupported_grant_type}, {@code invalid_scope}).
+ */
+public class OAuth2BadRequestException extends BadRequestException implements OAuth2Error {
Review Comment:
I dont think it has to be explicitly defined in the spec.
This is just a library api change for the Java library. The wire protocol (spec) is still the same. It just lets downstream code read the error type instead of parsing the exception message.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org