Skip to content

feat(block-explorers): add no_proxy to ClientBuilder#51

Open
coderHarrii wants to merge 3 commits intofoundry-rs:mainfrom
coderHarrii:proxy-no-proxy
Open

feat(block-explorers): add no_proxy to ClientBuilder#51
coderHarrii wants to merge 3 commits intofoundry-rs:mainfrom
coderHarrii:proxy-no-proxy

Conversation

@coderHarrii
Copy link
Copy Markdown

Summary

Mirrors [reqwest::ClientBuilder::no_proxy] on foundry_block_explorers::ClientBuilder, letting callers disable system proxy detection without having to construct their own reqwest::Client.

Motivation

In sandboxed environments (e.g., Cursor IDE, macOS App Sandbox, restricted shells) reqwest's system proxy lookup via SCDynamicStore can crash with:

The application panicked (crashed).
Message:  Attempted to create a NULL object.
Location: .../system-configuration-0.6.1/src/dynamic_store.rs:154

foundry-rs/foundry#13155 already added a --no-proxy flag for the RPC transport. foundry-rs/foundry#14454 does the same for the Etherscan path, but currently has to construct a reqwest::Client manually and pipe it in via with_client(..), which forces foundry-config to pull in reqwest directly.

This PR moves the primitive to the right layer so the foundry-side fix can become a single .no_proxy() call.

Changes

  • Add no_proxy: bool field to ClientBuilder.
  • Add ClientBuilder::no_proxy() method mirroring reqwest::ClientBuilder::no_proxy.
  • When a custom client is supplied via with_client, no_proxy is ignored (user-provided client wins).
  • Unit test for .no_proxy().build().

Follow-up

foundry-rs/foundry#14454 will be updated to call .no_proxy() and drop its direct reqwest dependency once this ships.

Mirrors `reqwest::ClientBuilder::no_proxy`, letting callers disable
system proxy detection without having to construct their own
`reqwest::Client`. This is useful in sandboxed environments where
`reqwest`'s system proxy lookup can panic (for example on macOS when
`SCDynamicStore` returns NULL).

Has no effect when a custom client is supplied via `with_client`.

Refs: foundry-rs/foundry#12733
Copy link
Copy Markdown
Collaborator

@mablr mablr left a comment

Choose a reason for hiding this comment

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

makes sense, ty!

please run cargo +nightly fmt

@coderHarrii coderHarrii requested a review from mablr April 24, 2026 19:48
Copy link
Copy Markdown
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

pending @mablr

Comment thread crates/explorers/block-explorers/src/lib.rs
Copy link
Copy Markdown
Member

@DaniPopes DaniPopes left a comment

Choose a reason for hiding this comment

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

how about a method that takes reqwest clientbuilder directly?
we have .with_client - can't this be used?

@mablr
Copy link
Copy Markdown
Collaborator

mablr commented Apr 24, 2026

we have .with_client - can't this be used?

@DaniPopes Indeed, this was submitted here: foundry-rs/foundry#14454

However, it introduces a new reqwest dependency on the consumer (foundry) that needs to be synchronized with the version of foundry-core. This often becomes messy, which is why I initially proposed the idea to mirror the reqwest::ClientBuilder API as we wrap it. In my opinion, this doesn’t hurt.
foundry-rs/foundry#14454 (review)

@DaniPopes
Copy link
Copy Markdown
Member

there should only be one version anyway, we import reqwest everywhere anyway

@coderHarrii coderHarrii requested a review from mattsse April 25, 2026 05:47
Copy link
Copy Markdown
Collaborator

@mablr mablr left a comment

Choose a reason for hiding this comment

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

g2g if @DaniPopes has no objection

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.

5 participants