Skip to content

fix: remove false-positive HTTPS_PROXY warning when HTTPS_PROXY is already set - #1839

Open
pranav-new-relic wants to merge 1 commit into
mainfrom
fix/https-proxy-false-positive-warning
Open

fix: remove false-positive HTTPS_PROXY warning when HTTPS_PROXY is already set#1839
pranav-new-relic wants to merge 1 commit into
mainfrom
fix/https-proxy-false-positive-warning

Conversation

@pranav-new-relic

@pranav-new-relic pranav-new-relic commented May 5, 2026

Copy link
Copy Markdown
Member

Summary

  • The guided install was printing a warning "Please ensure the HTTPS_PROXY environment variable is set" even when HTTPS_PROXY was already set with an http:// URL (e.g. HTTPS_PROXY=http://localhost:8080)
  • Root cause: the condition used a broken string-matching heuristic — it checked if HTTPS_PROXY contained "http" but not "https", which matches any http:// URL
  • Fix: replaced the heuristic with a direct env var check — the warning now only fires when HTTP_PROXY is set but HTTPS_PROXY is absent (the actual misconfiguration the message was meant to flag)
  • Replaced the non-asserting TestProxyNetwork (which only called t.Log) with TestProxyWarningCondition, which covers all four cases with real assertions

Test plan

  • go test -tags unit ./internal/install/ -run TestProxyWarningCondition -v — all 4 subtests pass
  • Manual: set HTTPS_PROXY=http://localhost:8080 and run guided install — warning should no longer appear
  • Manual: set HTTP_PROXY=http://localhost:8080 (no HTTPS_PROXY) and run guided install — warning should still appear

Fixes NR-560830 — resolved via AI (Claude Code / Claude Sonnet 4.6)

🤖 Generated with Claude Code

@pranav-new-relic
pranav-new-relic force-pushed the fix/https-proxy-false-positive-warning branch from 55a75ac to 48dfa5f Compare May 5, 2026 11:14
@codecov

codecov Bot commented May 5, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 31.32%. Comparing base (4ed200d) to head (48dfa5f).
⚠️ Report is 33 commits behind head on main.

Files with missing lines Patch % Lines
internal/install/command.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1839      +/-   ##
==========================================
+ Coverage   31.30%   31.32%   +0.01%     
==========================================
  Files         181      181              
  Lines       12551    12588      +37     
==========================================
+ Hits         3929     3943      +14     
- Misses       8478     8504      +26     
+ Partials      144      141       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…ready set

The previous condition used string matching to detect whether HTTPS_PROXY
was set to an http:// URL and warned the user — but HTTPS_PROXY=http://...
is a valid and working configuration (the HTTPS in the var name refers to
the proxied traffic type, not the proxy connection scheme).

The warning now only fires when HTTP_PROXY is set but HTTPS_PROXY is absent,
which is the actual misconfiguration the message was intended to flag.

Fixes NR-560830

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@pranav-new-relic
pranav-new-relic force-pushed the fix/https-proxy-false-positive-warning branch from 48dfa5f to 260a099 Compare May 5, 2026 11:30
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.

1 participant