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: error handling for TS SDK, GSon body serialization for Kotlin SDK #601

Merged
merged 3 commits into from
Apr 20, 2021

Conversation

jkaster
Copy link
Contributor

@jkaster jkaster commented Apr 19, 2021

We have an outstanding feature request to improve analysis of Kotlin SDK errors, but this documents the encountered error in the Typescript SDK test, which has better error handling

API Explorer and the existing Looker API Docs UI can also be used to see what the actual 422 error is

Fixes #544 🦕

For diagnosing #544

We have an outstanding feature request to improve analysis of Kotlin SDK errors, but this documents the encountered error in the Typescript SDK test, which has better error handling

API Explorer and the existing Looker API Docs UI can be used to see what the actual 422 error is
@google-cla google-cla bot added the cla: yes label Apr 19, 2021
@jkaster jkaster changed the title Jk/sdk error handling chore: tests for create_user_attribute Apr 19, 2021
@github-actions

This comment has been minimized.

Gson strips `null` properties from the JSON body so create_user_attribute no longer throws an error

Updated test to verify correct behavior and report an error if the call fails

Fixes #544
@jkaster jkaster changed the title chore: tests for create_user_attribute fix: error handling for TS SDK, GSon body serialization for Kotlin SDK Apr 20, 2021
@github-actions
Copy link
Contributor

Test Results

    7 files    73 suites   4m 8s ⏱️
152 tests 152 ✔️ 0 💤 0 ❌
532 runs  532 ✔️ 0 💤 0 ❌

Results for commit 881853b.

Copy link
Contributor

@joeldodge79 joeldodge79 left a comment

Choose a reason for hiding this comment

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

I'm kind of feeling like this should be two separate PRs for the sake of cleaner changelog entries? Granted right now we're not maintaining a kotlin changelog (hopefully will be soon). As written, the packages/sdk/CHANGELOG.md entry will be:

### Fixes
* error handling for TS SDK, GSon body serialization for Kotlin SDK

But if you're ok with that then we can merge as-is.

@jkaster
Copy link
Contributor Author

jkaster commented Apr 20, 2021

The PR was based on analyzing and understanding what was happening with the endpoint. I could make separate PRs but since the changelog won't get processed for the Kotlin SDK right now I'd rather not spend the time.

@joeldodge79 joeldodge79 self-requested a review April 20, 2021 21:10
@jkaster jkaster merged commit 11e924f into main Apr 20, 2021
@jkaster jkaster deleted the jk/sdk_error_handling branch April 20, 2021 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot create User / Group Attributes with Kotlin SDK if User Attribute is not hidden
2 participants