Skip to content
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

feat: improve handling when crossing async and sync boundaries #2976

Merged
merged 4 commits into from
May 24, 2021

Conversation

igorbernstein2
Copy link
Collaborator

Previously we would blindly uses ApiExceptions.callAndTranslateApiException and Futures.getUnchecked to await future results. This created a number of problems:

  • It would bubble ApiExceptions & CheckExceptions to the surface, which is quite unexpected for an hbase client
  • It would hide hbase specific IOExceptions

This PR introduces FutureUtil#unwrap, which will try to preserve the original exception. However to aid in debugging, it will augment the stacktrace to include the caller. This is a similar approach as hbase took in:
https://github.com/apache/hbase/blob/5b9940907e695e20d24968cbc725277a19ce2170/hbase-common/src/main/java/org/apache/hadoop/hbase/util/FutureUtils.java#L129-L140

Unfortunately we can't just call that utility because it only exists in hbase 2x, so I inlined relevant bits

Also, a couple methods would skip the HBaseIOException wrapping, so I patched that in

@igorbernstein2 igorbernstein2 requested a review from a team as a code owner May 19, 2021 16:36
@product-auto-label product-auto-label bot added the api: bigtable Issues related to the googleapis/java-bigtable-hbase API. label May 19, 2021
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label May 19, 2021
@igorbernstein2 igorbernstein2 requested a review from kolea2 May 19, 2021 16:37
igorbernstein2 added a commit to igorbernstein2/cloud-bigtable-client that referenced this pull request May 19, 2021
@igorbernstein2 igorbernstein2 requested a review from a team as a code owner May 19, 2021 20:34
@igorbernstein2 igorbernstein2 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 20, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 20, 2021
@igorbernstein2 igorbernstein2 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 20, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 20, 2021
Previously we would blindly uses ApiExceptions.callAndTranslateApiException and Futures.getUnchecked to await future results. This created a number of problems:
* It would bubble ApiExceptions & CheckExceptions to the surface, which is quite unexpected for an hbase client
* It would hide hbase specific IOExceptions

This PR introduces FutureUtil#unwrap, which will try to preserve the original exception. However to aid in debugging, it will augment the stacktrace to include the caller. This is a similar approach as hbase took in:
https://github.com/apache/hbase/blob/5b9940907e695e20d24968cbc725277a19ce2170/hbase-common/src/main/java/org/apache/hadoop/hbase/util/FutureUtils.java#L129-L140

Unfortunately we can't just call that utility because it only exists in hbase 2x, so I inlined relevant bits
@igorbernstein2 igorbernstein2 merged commit 0a63256 into googleapis:master May 24, 2021
@igorbernstein2 igorbernstein2 deleted the errors branch May 24, 2021 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigtable Issues related to the googleapis/java-bigtable-hbase API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants