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

Error responses from Client should include code #17

Closed
haruska opened this issue Apr 10, 2024 · 1 comment
Closed

Error responses from Client should include code #17

haruska opened this issue Apr 10, 2024 · 1 comment

Comments

@haruska
Copy link
Contributor

haruska commented Apr 10, 2024

Without a code included, downstream clients need to parse the error message to understand what the error was.

austin-denoble added a commit that referenced this issue Jun 7, 2024
## Problem
We're currently not including the code with error responses from
`Client`. Additionally, we don't seem to be gracefully handling possible
`ErrorResponse` bodies alongside more generic JSON and string responses.

There's an issue tracking this problem here:
#17

Ultimately, we want to surface more useful information to users through
errors.

## Solution
This PR focuses on improving the errors that are returned from REST
operations in `Client` methods.

- Decode the `http.Response` `Body` once, and then use the `[]byte`
value to decode into either errors or response structs.
- Update `decodeIndex` and `decodeCollection` to take in `[]byte` rather
than `io.ReadCloser`. Most of this was done to avoid reading the body to
EOF.
- Add `errorResponseMap` which is used to build up the error
information.
- Add `decodeErrorResponse`, `handleErrorResponseBody`, and
`formatError` to deal with parsing error payloads and turning them into
more readable output.
- Add `PineconeError` which extends the `error` interface and wraps
errors in a struct consumers are able to use if they'd like via
reflection.

## Type of Change
- [ ] Bug fix (non-breaking change which fixes an issue)
- [X] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] This change requires a documentation update
- [ ] Infrastructure change (CI configs, etc)
- [ ] Non-code change (docs, etc)
- [ ] None of the above: (explain here)

## Test Plan
Unit tests and a few integration tests have been added to validate
`handleErrorResponseBody` and that we're returning `PineconeError` from
failed network operations.


---
- To see the specific tasks where the Asana app for GitHub is being
used, see below:
  - https://app.asana.com/0/0/1207208972408344
@austin-denoble
Copy link
Contributor

In v0.5.0 we've added more robust error handling, which should allow easier access to code. See this PR: #23

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

No branches or pull requests

2 participants