Skip to content

jsonclient: surface HTTP Do and Read errors#1695

Merged
mhutchinson merged 1 commit intogoogle:masterfrom
FiloSottile:master
May 6, 2025
Merged

jsonclient: surface HTTP Do and Read errors#1695
mhutchinson merged 1 commit intogoogle:masterfrom
FiloSottile:master

Conversation

@FiloSottile
Copy link
Copy Markdown
Contributor

Right now they are shadowed by the Close error.

Note this from the net/http Client.Do docs, which allows simplifying the
logic by far:

On error, any Response can be ignored. A non-nil Response with a
non-nil error only occurs when CheckRedirect fails, and even then the
returned [Response.Body] is already closed.

@FiloSottile FiloSottile requested a review from a team as a code owner May 5, 2025 19:30
@FiloSottile FiloSottile requested review from mhutchinson and removed request for a team May 5, 2025 19:30
@roger2hk
Copy link
Copy Markdown
Contributor

roger2hk commented May 5, 2025

/gcbrun

Right now they are shadowed by the Close error.

Note this from the net/http Client.Do docs, which allows simplifying the
logic by far:

> On error, any Response can be ignored. A non-nil Response with a
> non-nil error only occurs when CheckRedirect fails, and even then the
> returned [Response.Body] is already closed.
@roger2hk
Copy link
Copy Markdown
Contributor

roger2hk commented May 5, 2025

/gcbrun

@mhutchinson mhutchinson merged commit bc7acd8 into google:master May 6, 2025
7 checks passed
mcpherrinm added a commit to letsencrypt/boulder that referenced this pull request May 6, 2025
This updates to current `master`, bc7acd89f703743d050f5cd4a3b9746808e0fdae

Notably, it includes a bug-fix to error handling in the HTTP client, which we
found was hiding errors from CT logs, hindering our debugging.

That fix is google/certificate-transparency-go#1695

No release has been tagged since this PR merged, so using the `master` commit.

A few mutual dependencies used by both Boulder and ct-go are updated, including
mysql, otel, and grpc.
aarongable pushed a commit to letsencrypt/boulder that referenced this pull request May 6, 2025
This updates to current `master`,
bc7acd89f703743d050f5cd4a3b9746808e0fdae

Notably, it includes a bug-fix to error handling in the HTTP client,
which we found was hiding errors from CT logs, hindering our debugging.

That fix is
google/certificate-transparency-go#1695

No release has been tagged since this PR merged, so using the `master`
commit.

A few mutual dependencies used by both Boulder and ct-go are updated,
including mysql, otel, and grpc.
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.

3 participants