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

Adapter/Guzzle: Fix error handling for v4 API #164

Merged
merged 2 commits into from
May 28, 2021

Conversation

deuill
Copy link
Contributor

@deuill deuill commented Mar 10, 2021

This commit represents a partial overhaul of error handling for requests
made against the v4 Cloudflare API, with an aim of unifying disparate
kinds of exceptions under a single ResponseException type, and the
covering of additional cases where errors were unhandled. Specifically:

  • The Guzzle::request() function will now catch Guzzle exceptions
    normally thrown in cases of client and server errors (4xx and 5xx)
    response codes, and convert these to ResponseException types
    before re-throwing. These types of errors were previously not caught
    and were instead returned verbatim, expecting downstream clients to
    be aware of internal details of how these functions operate.

  • Conversely, we no longer assume that all responses are JSON-encoded,
    and no longer try to derive errors from non-4xx or 5xx responses.
    All public endpoints under the v4 API are expected to be
    well-behaved in that regard, and never return an error response
    where none is indicated in the HTTP code.

Code has been moved around and test-cases added in support of these
changes. In most cases, these changes won't break any existing
expectations and won't require any changes to downstream code, but users
of the Cloudflare SDK should ensure that they are indeed set up for
catching ResponseException instances thrown during requests, and
should not expect to see Guzzle exceptions directly (though these are
still available in calls to ResponseException::getPrevious()).

Fixes: #152

This commit represents a partial overhaul of error handling for requests
made against the v4 Cloudflare API, with an aim of unifying disparate
kinds of exceptions under a single `ResponseException` type, and the
covering of additional cases where errors were unhandled. Specifically:

  - The `Guzzle::request()` function will now catch Guzzle exceptions
    normally thrown in cases of client and server errors (4xx and 5xx)
    response codes, and convert these to `ResponseException` types
    before re-throwing. These types of errors were previously not caught
    and were instead returned verbatim, expecting downstream clients to
    be aware of internal details of how these functions operate.

  - Conversely, we no longer assume that all responses are JSON-encoded,
    and no longer try to derive errors from non-4xx or 5xx responses.
    All public endpoints under the v4 API are expected to be
    well-behaved in that regard, and never return an error response
    where none is indicated in the HTTP code.

Code has been moved around and test-cases added in support of these
changes. In most cases, these changes won't break any existing
expectations and won't require any changes to downstream code, but users
of the Cloudflare SDK should ensure that they are indeed set up for
catching `ResponseException` instances thrown during requests, and
should not expect to see Guzzle exceptions directly (though these are
still available in calls to `ResponseException::getPrevious()`).

Fixes: #152
@jacobbednarz jacobbednarz force-pushed the apalaistras/fix-request-error-handling branch from c4201ed to c58f340 Compare May 28, 2021 00:23
@jacobbednarz jacobbednarz merged commit 1e58a65 into master May 28, 2021
@jacobbednarz jacobbednarz deleted the apalaistras/fix-request-error-handling branch May 28, 2021 00:49
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

Successfully merging this pull request may close these issues.

SDK is broken: trying to get KV value throws exception
2 participants