Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

fix: use new http transport for webhook handler#19462

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
dannykopping merged 1 commit intomainfromdk/flake-919
Aug 21, 2025

Conversation

dannykopping
Copy link
Contributor

@dannykoppingdannykopping commentedAug 21, 2025
edited
Loading

Closescoder/internal#919

I believe thatError "request failed: Post \"http://127.0.0.1:46685\": net/http: HTTP/1.x transport connection broken: http: CloseIdleConnections called" does not contain "non-2xx response (500)" indicates that the client is closing what it considers to be an idle connection while another goroutine is using it. This is likely caused by another test (running concurrently) shutting down its http server while this test was still running, and both shared a reference to the globalhttp.DefaultTransport.http.Transport maintains a connection pool.

Now we'll be cloninghttp.DefaultTransport whenever a new webhook handler gets created, ensuring that it maintains its own conn pool.

I've added some defensiveness for the type inference, but it's highly unlikely that will change; in which case let's just revert to previous behaviour and log. I considered copying the panic pattern fromtailnet.go, but since this webhook feature is optional, I think it's better to choose a slightly racy implementation than crash the server.

… pool, avoiding conflicts over shared connectionsSigned-off-by: Danny Kopping <dannykopping@gmail.com>
@dannykoppingdannykopping marked this pull request as ready for reviewAugust 21, 2025 09:50
Copy link
Member

@johnstcnjohnstcn left a comment

Choose a reason for hiding this comment

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

Should we have some kind of internal "helper" library, perhaps something similar to github.com/hashicorp/go-cleanhttp?

@dannykopping
Copy link
ContributorAuthor

I like that idea. It'll be quite a sizeable/wide-ranging refactor I'd imagine, and out of scope for this PR, but probably something we should look into in the future.

Thanks for the quick review!

@dannykoppingdannykopping merged commit338e8b5 intomainAug 21, 2025
35 checks passed
@dannykoppingdannykopping deleted the dk/flake-919 branchAugust 21, 2025 10:08
@github-actionsgithub-actionsbot locked and limited conversation to collaboratorsAug 21, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@johnstcnjohnstcnjohnstcn approved these changes

Assignees

@dannykoppingdannykopping

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

flake: TestWebhook/non-200_response
2 participants
@dannykopping@johnstcn

[8]ページ先頭

©2009-2025 Movatter.jp