tigrisdata-archive / tigris-client-ts

TypeScript client for Tigris
https://www.tigrisdata.com/docs/sdkstools/typescript/
Apache License 2.0
15 stars 10 forks source link

chore: Improve error logging when project name in use does not exist on Tigris #378

Closed jemiluv8 closed 1 year ago

jemiluv8 commented 1 year ago

Background

Some calls to the server return useful error messages by default. A few operations do not. The initializeBranch functionality for instance, explicitly handles similar errors and logs a useful error message when branch already exists.

Since any operation on a non-existing project will fail I explored the possibility of finding a "central" location to log a useful error message. I didn't find any. My only thought was to modify the response returned by the "server"

In this PR I only modified the createBranch method to log a useful error message when project does not exist. The same method is called when initializing branches so this change caters for both.

Other methods (registerSchemas) where similar errors occur have useful error messages returned from the api at the moment.

What type of PR is this? (check all applicable)

Description

Related Tickets & Documents

Added/updated tests?

Is this change backwards compatible?

Does it require updates to Tigris docs?

Checklist

[optional] Are there any post deployment tasks we need to perform?

/claim #377

reviewpad[bot] commented 1 year ago

Reviewpad Report

:bangbang: Errors

modify error code assertion in createBranch to rely on Status.NOT_FOUND instead of Status.UNKNOWN' (7920e79fd7423034dcd19dcecf0a425b1c13d626)

adilansari commented 1 year ago

Thanks for fixing this @jemiluv8, added minor comments.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 25.00% and project coverage change: -0.05 :warning:

Comparison is base (267e12d) 90.26% compared to head (6efcaa4) 90.21%.

:exclamation: Current head 6efcaa4 differs from pull request most recent head 7920e79. Consider uploading reports for the commit 7920e79 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #378 +/- ## ========================================== - Coverage 90.26% 90.21% -0.05% ========================================== Files 29 29 Lines 5606 5610 +4 Branches 658 659 +1 ========================================== + Hits 5060 5061 +1 - Misses 545 548 +3 Partials 1 1 ``` | [Impacted Files](https://app.codecov.io/gh/tigrisdata/tigris-client-ts/pull/378?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tigrisdata) | Coverage Δ | | |---|---|---| | [src/db.ts](https://app.codecov.io/gh/tigrisdata/tigris-client-ts/pull/378?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tigrisdata#diff-c3JjL2RiLnRz) | `93.08% <25.00%> (-0.62%)` | :arrow_down: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

jemiluv8 commented 1 year ago

@adilansari I added the test for the branch creation and used the Status.NOT_FOUND instead of Status.UKNOWN. But like I reiterated in my earlier comment, I got Status.UNKNOWN when testing against an actual tigris server using the current version of the library.

adilansari commented 1 year ago

@adilansari I added the test for the branch creation and used the Status.NOT_FOUND instead of Status.UKNOWN. But like I reiterated in my earlier comment, I got Status.UNKNOWN when testing against an actual tigris server using the current version of the library.

Sounds good, replied in other comment here. I'll accept this and fix the bug on server side.

github-actions[bot] commented 1 year ago

:tada: This PR is included in version 1.3.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: