Allow proxy configuration via HTTP_PROXY env var (#2161)

* Allow proxy config via environment

* Unit tests for env[http_proxy] support

* Update sentry-ruby/spec/sentry/transport/http_transport_spec.rb

Co-authored-by: Stan Lo <[email protected]>

* Add transport/configuration comments

* Fixed Ruby version checks for proxy conf

---------

Co-authored-by: Stan Lo <[email protected]>

CHANGELOG.md

@@ -46,6 +46,7 @@
 - Fix breadcrumbs with `warn` level not being ingested [#2150](https://github.com/getsentry/sentry-ruby/pull/2150)
   - Fixes [#2145](https://github.com/getsentry/sentry-ruby/issues/2145)
 - Don't send negative line numbers in profiles [#2158](https://github.com/getsentry/sentry-ruby/pull/2158)
+- Allow transport proxy configuration to be set with `HTTP_PROXY` environment variable [#2161](https://github.com/getsentry/sentry-ruby/pull/2161)
 
 ## 5.12.0
 

sentry-ruby/lib/sentry/transport/configuration.rb

@@ -3,7 +3,80 @@
 module Sentry
   class Transport
     class Configuration
-      attr_accessor :timeout, :open_timeout, :proxy, :ssl, :ssl_ca_file, :ssl_verification, :encoding
+
+      # The timeout in seconds to open a connection to Sentry, in seconds.
+      # Default value is 2.
+      #
+      # @return [Integer]
+      attr_accessor :timeout
+
+      # The timeout in seconds to read data from Sentry, in seconds.
+      # Default value is 1.
+      #
+      # @return [Integer]
+      attr_accessor :open_timeout
+
+      # The proxy configuration to use to connect to Sentry.
+      # Accepts either a URI formatted string, URI, or a hash with the `uri`,
+      # `user`, and `password` keys.
+      #
+      # @example
+      #   # setup proxy using a string:
+      #   config.transport.proxy = "https://user:password@proxyhost:8080"
+      #
+      #   # setup proxy using a URI:
+      #   config.transport.proxy = URI("https://user:password@proxyhost:8080")
+      #
+      #   # setup proxy using a hash:
+      #   config.transport.proxy = {
+      #     uri: URI("https://proxyhost:8080"),
+      #     user: "user",
+      #     password: "password"
+      #   }
+      #
+      # If you're using the default transport (`Sentry::HTTPTransport`),
+      # proxy settings will also automatically be read from tne environment
+      # variables (`HTTP_PROXY`, `HTTPS_PROXY`, `NO_PROXY`).
+      #
+      # @return [String, URI, Hash, nil]
+      attr_accessor :proxy
+
+      # The SSL configuration to use to connect to Sentry.
+      # You can either pass a `Hash` containing `ca_file` and `verification` keys,
+      # or you can set those options directly on the `Sentry::HTTPTransport::Configuration` object:
+      #
+      # @example
+      #   config.transport.ssl =  {
+      #     ca_file: "/path/to/ca_file",
+      #     verification: true
+      #   end
+      #
+      # @return [Hash, nil]
+      attr_accessor :ssl
+
+      # The path to the CA file to use to verify the SSL connection.
+      # Default value is `nil`.
+      #
+      # @return [String, nil]
+      attr_accessor :ssl_ca_file
+
+      # Whether to verify that the peer certificate is valid in SSL connections.
+      # Default value is `true`.
+      #
+      # @return [Boolean]
+      attr_accessor :ssl_verification
+
+      # The encoding to use to compress the request body.
+      # Default value is `Sentry::HTTPTransport::GZIP_ENCODING`.
+      #
+      # @return [String]
+      attr_accessor :encoding
+
+      # The class to use as a transport to connect to Sentry.
+      # If this option not set, it will return `nil`, and Sentry will use
+      # `Sentry::HTTPTransport` by default.
+      #
+      # @return [Class, nil]
       attr_reader :transport_class
 
       def initialize

sentry-ruby/lib/sentry/transport/http_transport.rb

@@ -128,12 +128,15 @@ def should_compress?(data)
 
     def conn
       server = URI(@dsn.server)
-
+
+      # connection respects proxy setting from @transport_configuration, or environment variables (HTTP_PROXY, HTTPS_PROXY, NO_PROXY)
+      # Net::HTTP will automatically read the env vars.
+      # See https://ruby-doc.org/3.2.2/stdlibs/net/Net/HTTP.html#class-Net::HTTP-label-Proxies
       connection =
         if proxy = normalize_proxy(@transport_configuration.proxy)
           ::Net::HTTP.new(server.hostname, server.port, proxy[:uri].hostname, proxy[:uri].port, proxy[:user], proxy[:password])
         else
-          ::Net::HTTP.new(server.hostname, server.port, nil)
+          ::Net::HTTP.new(server.hostname, server.port)
         end
 
       connection.use_ssl = server.scheme == "https"
@@ -148,6 +151,9 @@ def conn
       connection
     end
 
+    # @param proxy [String, URI, Hash] Proxy config value passed into `config.transport`.
+    #   Accepts either a URI formatted string, URI, or a hash with the `uri`, `user`, and `password` keys.
+    # @return [Hash] Normalized proxy config that will be passed into `Net::HTTP`
     def normalize_proxy(proxy)
       return proxy unless proxy
 

sentry-ruby/spec/sentry/transport/http_transport_spec.rb

@@ -63,7 +63,7 @@
         expect(subject.send(:conn).port).to eq(80)
       end
     end
-    context "with http DSN" do
+    context "with https DSN" do
       let(:dsn) { "https://12345:[email protected]/sentry/42" }
 
       it "sets port to 443" do
@@ -129,13 +129,33 @@
       subject.send_data(data)
     end
 
+    it "accepts a proxy from ENV[HTTP_PROXY]" do
+      begin
+        ENV["http_proxy"] = "https://stan:[email protected]:8080"
+
+        stub_request(fake_response) do |_, http_obj|
+          expect(http_obj.proxy_address).to eq("example.com")
+          expect(http_obj.proxy_port).to eq(8080)
+
+          if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new("2.5")
+            expect(http_obj.proxy_user).to eq("stan")
+            expect(http_obj.proxy_pass).to eq("foobar")
+          end
+        end
+
+        subject.send_data(data)
+      ensure
+        ENV["http_proxy"] = nil
+      end
+    end
+
     it "accepts custom timeout" do
       configuration.transport.timeout = 10
 
       stub_request(fake_response) do |_, http_obj|
         expect(http_obj.read_timeout).to eq(10)
 
-        if RUBY_VERSION >= "2.6"
+        if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new("2.6")
           expect(http_obj.write_timeout).to eq(10)
         end
       end