Skip to content
This repository has been archived by the owner on Sep 4, 2024. It is now read-only.

minreq_http: return an HTTP error on error with no JSON in body #103

Merged
merged 1 commit into from
Jun 28, 2023

Conversation

darosior
Copy link
Contributor

It is useful for downstream users to be matching on errors that do not contain a valid JSONRPC error in the HTTP response body. One instance is if the HTTP server work queue depth is exceeded, as they probably want to retry the request later.

On such error we would return a JSON deserialization error, without exposing neither the HTTP status code nor the body of the response. This made it impossible to detect such transient errors.

Instead, introduce a new HttpError variant that gets returned when the requested is responded to by an error that does not contain valid JSON in its body.

See also: #94 (comment).

It is useful for downstream users to be matching on errors that do not
contain a valid JSONRPC error in the HTTP response body. One instance is
if the HTTP server work queue depth is exceeded, as they probably want
to retry the request later.

On such error we would return a JSON deserialization error, without
exposing neither the HTTP status code nor the body of the response. This
made it impossible to detect such transient errors.

Instead, introduce a new HttpError variant that gets returned when the
requested is responded to by an error that does not contain valid JSON
in its body.
Copy link
Owner

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 66fb441

@apoelstra apoelstra merged commit 090dc0f into apoelstra:master Jun 28, 2023
9 checks passed
@darosior darosior deleted the non_json_errors branch June 29, 2023 13:08
apoelstra added a commit that referenced this pull request Jun 29, 2023
5b34d4a Bump version to 16.0 (Antoine Poinsot)
223d556 simple_http: gate FromStr usage behind 'proxy' feature (Antoine Poinsot)
d57ee9d minreq_http: make Error enum #[non_exhaustive] (Antoine Poinsot)

Pull request description:

  To make #103 and #102 available downstream. Note i've successfully tested #103 in my software (detect and retry requests to bitcoind upon hitting a transient workqueue exceeded error).

  Updating the Error enum for minreq_http is an API break, so bump the major version.

ACKs for top commit:
  tcharding:
    ACK 5b34d4a
  apoelstra:
    ACK 5b34d4a

Tree-SHA512: f19641b39da346d61cf4f4f3c1f47bf39dd81394c26f88043ae92d30124583e74f2d61b465ef42c157e6a93fc757fc6d396aeb716c7d8347a0b48bed5a9924d3
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants