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

Generic response error handling #570

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Conversation

Fank
Copy link
Contributor

@Fank Fank commented Sep 13, 2022

What type of PR is this?

  • cleanup
  • feature
  • api-change

What this PR does / why we need it:

This PR replaces the existing request error handling with a more generic variation which allows it to extract the response body depending on the use case and handle the status code which isn't possible at the moment.

Which issue(s) this PR fixes:

Relates to #565 (comment)

Special notes for your reviewer:

I am not familiar with any kind of error messages, so far i only checked the insight API and there errors messages are not static or being able to parse to static structs.

Additional documentation e.g., usage docs, etc.:

The idea was to have use cases like:

issue, _, err := client.Issue.Get(context.Background(), "123", nil)
if err != nil && errors.Is(err, ErrNotFound) {
	// create new issue
} else err != nil {
	return err
}

issue, _, err := client.Issue.Create(context.Background(), &Issue{})
if err != nil && errors.Is(err, ErrValidation) {
	// check error content and proceed as preferred
} else err != nil {
	return err
}

@Fank Fank mentioned this pull request Sep 13, 2022
2 tasks
@github-actions github-actions bot added the conflicts Indicates merge conflicts label Oct 19, 2022
@andygrunwald
Copy link
Owner

Thanks for this PR. I might take some time to review this, but it is still on my list.

One quick question:
Do you have any particular things in mind that need to be done before this PR can move out of the Draft state?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conflicts Indicates merge conflicts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants