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

[BUG]: HTTP or API errors are not available in any way #20

Open
braaar opened this issue Oct 25, 2023 · 0 comments
Open

[BUG]: HTTP or API errors are not available in any way #20

braaar opened this issue Oct 25, 2023 · 0 comments
Labels
bug Something isn't working

Comments

@braaar
Copy link

braaar commented Oct 25, 2023

Version

v0.0.2

Endpoint

Any endpoint. I've tried Agent and Announcement

Expected Behavior

When running queries with invalid API credentials or bad request payloads, I would expect there to be some HTTP error returned (400, 401, 403, 404, etc) and sometimes a response body describing what went wrong. I've tried creating, listing and fetching individual entities, and it seems like in each case the HTTP error code and the error body is swallowed up internally in go-freshservice and is never available to me.

Actual Behavior

List queries return an empty slice [], while individual queries return nil. No errors are returned.

Steps to Reproduce

ctx := context.Background()

// I should get a 404 error here
api, err := fs.New(ctx, "fakedomain.freshservice.com", "badApikey", nil)
if err != nil {
    log.Fatal(err)
}
agents, _, agentsError := api.Agents().List(ctx, nil)

// agents is []
// agentsError is nil

Thoughts

This lack of access to HTTP error codes or error bodies almost seems to be by design when I take a look at the source code.

Even if the ErrorResponse interface is used conditionally in makeRequest to decode the response body, we always return &res.Details or &res.List in the query functions like Agents().List. Thus, the fields from ErrorResponse, namely Description and Errors are never used for anything and are simply discarded. The HTTP response status code is also never returned from makeRequest so that is definitely not available to me.

The existence of the ErrorResponse interface clearly indicates that HTTP errors, or at least freshservice API errors, are supposed to be handled somehow. But I'm not seeing how that can be done the way the queries are written today.

Is there something here I have misunderstood or is the codebase simply at such an early stage that error handling was never prioritised and every query is assumed to run successfully?

@braaar braaar added the bug Something isn't working label Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant