search

cargo-bins / cargo-binstall

Binary installation for rust projects
GNU General Public License v3.0
1.62k stars 60 forks source link

fix(gh_api_client): remote client should never being shared everywhere bacause the underlying connection pool will be reused. #1930

Closed Cryolitia closed 1 month ago

Cryolitia commented 1 month ago

The client pool contains connections associated with a runtime when the runtime closes, the client may panic at seeing the connection disappear "abruptly".

Reported-by: https://archriscv.felixc.at/.status/log.htm?url=logs/cargo-binstall/cargo-binstall-1.8.0-1.log Link: https://github.com/seanmonstar/reqwest/issues/1148

NobodyXu commented 1 month ago

Thank you!

Cryolitia commented 1 month ago

That's wired...I can't reproduce the unit test failure in CI on both my x86_64 and riscv64 device. I tried both cargo test and cargo nextest run

NobodyXu commented 1 month ago

https://github.com/cargo-bins/cargo-binstall/blob/a8227ae6a4b9d2175102621f0e87bd4ba504e74c/crates/binstalk-git-repo-api/src/gh_api_client.rs#L596

The CI failure is on this line, where we try to fetch private repository information and failed.

I suspect it's because you don't have permission to access our secrets because it's from a fork.

NobodyXu commented 1 month ago

cc @Cryolitia can you rebase against main please?

I've merged in a PR that should fix this error.

Cryolitia commented 1 month ago

cc @Cryolitia can you rebase against main please?

of course sure

NobodyXu commented 1 month ago

Thanks, now the CI is green 🎉