3.1.0: arm64 + proxy passthrough for binary download (LOC-6563)#57
Open
yashdsaraf wants to merge 4 commits into
Open
3.1.0: arm64 + proxy passthrough for binary download (LOC-6563)#57yashdsaraf wants to merge 4 commits into
yashdsaraf wants to merge 4 commits into
Conversation
- GetBinaryName made public and adds Linux arm64 branch via RuntimeInformation.OSArchitecture. arm64 wins over alpine to match the Node SDK ordering. - BrowserStackTunnel.SetProxy(host, port) + proxyHost/proxyPort fields. - fetchSourceUrl builds HttpClientHandler with Proxy = WebProxy(host, port) when proxy is set. - downloadBinary sets WebClient.Proxy = WebProxy(host, port) when set. (Keeping WebClient; proxy plumbing on it is one line.) - Local.addArgs captures proxyHost/proxyPort on local fields in addition to appending to argumentString. Local.start calls tunnel.SetProxy(...) before DownloadVerifyAndRunBinary. - Test fixtures derive binaryName from BrowserStackTunnel.GetBinaryName() so arm64 CI hosts pick the right binary. Tracks LOC-6563. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Local.cs addArgs: int.TryParse failure now logs to stderr instead of silently dropping the proxy on download with no signal to the user. - BrowserStackTunnel.cs: doc comment on proxyHost/proxyPort fields documents the required ordering (SetProxy before addBinaryPath). - BrowserStackTunnelTests.cs: 2 new unit tests covering GetBinaryName return-set and SetProxy no-throw contract. Adds the missing TunnelClass constructor that broke compile pre-this-PR. - LocalTests.cs: 3 new unit tests covering proxy-options-to-tunnel plumbing (with proxy, without proxy, with invalid port). Fixes pre-existing CS0854 errors on Mock.Verify expressions with optional args, and pre-existing missing-ctor errors on Mock<BrowserStackTunnel>(). Project now compiles for the first time in years; 5 new tests pass, 11 pre-existing tests still fail with pre-existing assertion bugs (unchanged behavior; CI does not run this project anyway). Code review findings #3, #4, #5 from LOC-6563 review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Matches what Node SDK already does on every HTTP request (rails endpoint POST AND CDN binary GET). Until this commit, the WebClient binary download was the only request without a UA header, which made CloudFront/CloudFlare access logs unfilterable by binding+version. Now every outbound HTTP from this binding sends: User-Agent: browserstack-local-csharp/<version> Same value the HttpClient already sent on the rails endpoint POST. Follow-up to the code review pass on LOC-6563. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Why: the 3.x binding's only net7-specific touchpoint is the `Architecture.Arm64` enum (added in .NET Core 3.0 / netstandard 2.1). Everything else (HttpClient, WebClient, WebProxy, Process, ACL APIs) is available since netstandard 2.0 / net6.0. Targeting net6.0 instead lets net6.0 SDK consumers stay on 3.x without being forced to upgrade their runtime to net7+. .NET 5 and netcoreapp 3.1 still aren't covered — but per the SDK agent's 7-day BigQuery, those TFMs have zero C# SDK traffic, so the practical impact is nil. Net effect: removes the LOC-6563 rollout's hardest constraint (forcing SDK to drop net6.0 from <TargetFrameworks> in order to adopt 3.1.0). The 7 hard-blocked enterprise accounts on .NET 6.0.36 (group_id=934837 et al., 14 users, 623 weekly events) stay supported. Also updates test project TFMs and HintPaths from net7.0 to net6.0 so the local build chain matches. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
rounak610
requested changes
Jun 2, 2026
| var url = "https://local.browserstack.com/binary/api/v1/endpoint"; | ||
|
|
||
| using (var client = new HttpClient()) | ||
| HttpClientHandler handler = new HttpClientHandler(); |
Collaborator
There was a problem hiding this comment.
in today's standup, it was decided to find alternative of HttpClientHandler so that we can support dotnet6 as well? are we going ahead with this library?
Collaborator
Author
There was a problem hiding this comment.
This is compatible already apparently. Not sure why this was called out as deprecated in binding version 3.0.0.
I ran a test e2e with dotnet 6.0 and it worked -- https://browserstack.atlassian.net/browse/LOC-6563?focusedCommentId=2082879
Collaborator
Author
There was a problem hiding this comment.
Added .net6.0 back in this commit d968e36
rounak610
approved these changes
Jun 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
RuntimeInformation.OSArchitecture == Architecture.Arm64selectsBrowserStackLocal-linux-arm64. arm64 wins over alpine to match the Node SDK ordering.proxyHost/proxyPortfromLocal.start(options)are now also threaded into the endpoint POST and the binary GET.fetchSourceUrl(HttpClient) anddownloadBinary(WebClient) both honor the proxy now. Users behind enterprise proxies can finally download.GetBinaryName()made public — test fixtures (BrowserStackTunnelTests.cs,IntegrationTests.cs) now use it as a single source of truth instead of hardcodedbinaryNameternaries.CloudFront migration was already done in this binding (see
fetchSourceUrl— already prod). This PR closes the remaining LOC-6563 items: arm64 + proxy.Background
Tracks LOC-6563. Ruby parity PR ships in coordination — see browserstack-local-ruby#39.
Files
Test plan
Pre-existing — NOT addressed in this PR
The `BrowserStackLocal Unit Tests` project does not compile in HEAD:
CS0854)Neither `.github/workflows/ci.yml` nor `.github/workflows/cd.yml` build or run this project — both target `BrowserStackLocalIntegrationTests` only. Left alone per scope; happy to file a follow-up.
🤖 Generated with Claude Code