You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2022/08/16 01:46:38 UTC
[GitHub] [kafka] hachikuji opened a new pull request, #12518: KAFKA-14167; Completion exceptions should not be translated directly to error codes
hachikuji opened a new pull request, #12518:
URL: https://github.com/apache/kafka/pull/12518
There are a few cases in `ControllerApis` where we may see an `ApiException` wrapped as a `CompletionException`. This can happen in `QuorumController.allocateProducerIds` where the returned future is the result of calling `thenApply` on the future passed to the controller. The danger when this happens is that the `CompletionException` gets passed to `Errors.forException`, which translates it to an `UNKNOWN_SERVER_ERROR`. At a minimum, I found that the `AllocateProducerIds` and `UpdateFeatures` APIs were affected by this bug, but it is difficult to root out all cases.
Interestingly, `DeleteTopics` was not affected as I originally suspected. This is because we have logic in `ApiError.fromThrowable` to check for both `CompletionException` and `ExecutionException` and to pull out the underlying cause. This patch moves this logic out of `ApiError.fromThrowable` and into `Errors.forException` to be sure that we handle all cases where exceptions are converted to error codes.
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] hachikuji commented on pull request #12518: KAFKA-14167; Completion exceptions should not be translated directly to error codes
Posted by GitBox <gi...@apache.org>.
hachikuji commented on PR #12518:
URL: https://github.com/apache/kafka/pull/12518#issuecomment-1218204897
@mumrah See the test case `ControllerApisTest.testAllocateProducerIdsReturnsNotController`. The underlying exception gets wrapped when it gets propagated to the future returned by `thenApply`.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] hachikuji merged pull request #12518: KAFKA-14167; Completion exceptions should not be translated directly to error codes
Posted by GitBox <gi...@apache.org>.
hachikuji merged PR #12518:
URL: https://github.com/apache/kafka/pull/12518
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] hachikuji commented on a diff in pull request #12518: KAFKA-14167; Completion exceptions should not be translated directly to error codes
Posted by GitBox <gi...@apache.org>.
hachikuji commented on code in PR #12518:
URL: https://github.com/apache/kafka/pull/12518#discussion_r948229463
##########
clients/src/main/java/org/apache/kafka/common/protocol/Errors.java:
##########
@@ -479,6 +482,19 @@ public static Errors forException(Throwable t) {
return UNKNOWN_SERVER_ERROR;
}
+ public static Throwable getCause(Throwable t) {
Review Comment:
Yeah, let me try to think of a better name. One idea I was toying with is letting this method search the exception chain for the first ApiException. Then it doesn't need to be specific to CompletionException or ExecutionException. What do you think about 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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] hachikuji commented on a diff in pull request #12518: KAFKA-14167; Completion exceptions should not be translated directly to error codes
Posted by GitBox <gi...@apache.org>.
hachikuji commented on code in PR #12518:
URL: https://github.com/apache/kafka/pull/12518#discussion_r948294284
##########
clients/src/main/java/org/apache/kafka/common/protocol/Errors.java:
##########
@@ -479,6 +482,19 @@ public static Errors forException(Throwable t) {
return UNKNOWN_SERVER_ERROR;
}
+ public static Throwable getCause(Throwable t) {
Review Comment:
I decided not to do this. There may be cases where we have intentionally wrapped an exception type because we don't want to expose the underlying error to the client.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mumrah commented on a diff in pull request #12518: KAFKA-14167; Completion exceptions should not be translated directly to error codes
Posted by GitBox <gi...@apache.org>.
mumrah commented on code in PR #12518:
URL: https://github.com/apache/kafka/pull/12518#discussion_r947938464
##########
clients/src/main/java/org/apache/kafka/common/protocol/Errors.java:
##########
@@ -479,6 +482,19 @@ public static Errors forException(Throwable t) {
return UNKNOWN_SERVER_ERROR;
}
+ public static Throwable getCause(Throwable t) {
Review Comment:
This name might be a little confusing since `Throwable#getCause` exists.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org