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

fix: Improve ok() helper function #1542

Merged
merged 2 commits into from
Dec 9, 2024
Merged

fix: Improve ok() helper function #1542

merged 2 commits into from
Dec 9, 2024

Conversation

haywood
Copy link
Collaborator

@haywood haywood commented Dec 3, 2024

I'm hesitant to change this function too much, since we don't know what kind of odd dependencies on its current behavior might exist. However, adding exception chaining and surfacing the original error message seems like a risk worth taking.

  • Eliminate duplicated version in AuthSession by delegating to a shared implementation defined on the SDKResponse companion object.

  • For the SDKError case:

    • Set Error.message from SDKError.message
    • Set Error.casue from SDKError.cause
  • Also ran ktlint -F on the files I changed so that gradle check would pass the linting stage

  • Also added kotlin-ci.yml to run ./gradlew jar on kotlin changes

Fixes #1539 🦕

- Eliminate duplicated version in AuthSession by delegating to a shared
  implementation defined on the SDKResponse companion object.

- For the SDKError case:
  - Set Error.message from SDKError.message
  - Set Error.casue from SDKError.cause
@haywood haywood force-pushed the haywood-improve-ok branch 6 times, most recently from 897905d to 608e1b5 Compare December 3, 2024 12:37
@haywood haywood marked this pull request as ready for review December 3, 2024 12:41
@haywood haywood requested a review from a team as a code owner December 3, 2024 12:41
@haywood haywood force-pushed the haywood-improve-ok branch 2 times, most recently from ad3f8e1 to 82c583b Compare December 3, 2024 12:51
@haywood haywood force-pushed the haywood-improve-ok branch from 82c583b to af708e2 Compare December 3, 2024 12:51
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.

Looks LGTM To Me

@haywood haywood merged commit 7b15f90 into main Dec 9, 2024
13 checks passed
@haywood haywood deleted the haywood-improve-ok branch December 9, 2024 14:40
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