Skip to content

Improve error handling in Transport.kt #1540

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

Merged
merged 2 commits into from
Dec 2, 2024
Merged

Improve error handling in Transport.kt #1540

merged 2 commits into from
Dec 2, 2024

Conversation

haywood
Copy link
Contributor

@haywood haywood commented Dec 2, 2024

  • Provide HTTP request and response details on SDKErrorResponse.
  • Provide the cause of SDKError for chaining and re-throwing
  • Add LookerApiException, which includes the same request/response information as SDKErrorResponse.
  • Mark ok() as Deprecated, because it throws java.lang.Error, which is not something application code should do.
  • Add SDKResponse::getOrThrow() instance method, patterned after Kotlin's built-in Result type, which handles SDKErrorResponse by throwing the newly introduced LookerApiException

👋👋 Thank you for contributing to Looker sdk-codegen (⚡️🍣)

  • 👆 Make sure your pull request title follows Pull Request Title Guidelines from our Contribution guide
  • 👉 Don't forget to replace these instructions with your ✨awesome✨ description of what this change actually does. Additionally, it's great to include context on how it works and why the change was needed.
  • 👇 Edit "Developer Checklist" to reflect items relevant to this PR (and try to make sure to check everything off before asking for review)

Developer Checklist ℹ️

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Appropriate docs were updated (if necessary)

Fixes #1539 🦕

- Provide HTTP request and response details on SDKErrorResponse.
- Provide the cause of SDKError for chaining and re-throwing
- Add LookerApiException, which includes the same request/response information as SDKErrorResponse.
- Mark ok() as Deprecated, because it throws java.lang.Error, which is not something application code should do.
- Add SDKResponse::getOrThrow() instance method, patterned after Kotlin's built-in Result type, which handles SDKErrorResponse by throwing the newly introduced LookerApiException
@haywood haywood marked this pull request as ready for review December 2, 2024 18:42
@haywood haywood requested a review from a team as a code owner December 2, 2024 18:42
Copy link
Collaborator

@drstrangelooker drstrangelooker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@haywood haywood merged commit 18add63 into main Dec 2, 2024
11 checks passed
@haywood haywood deleted the haywood-patch-1 branch December 2, 2024 18:43
drstrangelooker pushed a commit that referenced this pull request Dec 2, 2024
- Provide HTTP request and response details on SDKErrorResponse.
- Provide the cause of SDKError for chaining and re-throwing
- Add LookerApiException, which includes the same request/response
information as SDKErrorResponse.
- Mark ok() as Deprecated, because it throws java.lang.Error, which is
not something application code should do.
- Add SDKResponse::getOrThrow() instance method, patterned after
Kotlin's built-in Result type, which handles SDKErrorResponse by
throwing the newly introduced LookerApiException

Fixes #1539  🦕
drstrangelooker pushed a commit that referenced this pull request Dec 2, 2024
🤖 I have created a release *beep* *boop*
---


<details><summary>sdk-codegen-all: 24.20.1</summary>

##
[24.20.1](sdk-codegen-all-v24.20.0...sdk-codegen-all-v24.20.1)
(2024-12-02)


### Bug Fixes

* Improve error handling in Transport.kt
([#1540](#1540))
([a091549](a091549))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The SDK does not handle error conditions properly
2 participants