From cf6d79a1caa2f7385d583a3cf321a3bc5775d1be Mon Sep 17 00:00:00 2001 From: Brandur Date: Mon, 11 Feb 2019 11:14:27 -0800 Subject: [PATCH] Remove attempt to nicen `UploadIO` in logs In #741 I tried to do something too clever by replacing instances of `Faraday::UploadIO` found in parameters with a human-readable string to improve `STRIPE_LOG` logging output. I thought I'd tested it at the time, but apparently not (or not well enough), and this change caused the regression detailed in #742. My findings about how Faraday encodes multipart were apparently wrong and it does use these parameters, so here we remove the step where we try to nicen them for logging. The logs look a little worse, but it's not that big of a deal. I've tested this patch against the API and confirmed that it addresses the problem. Fixes #742. --- lib/stripe/stripe_client.rb | 22 +--------------------- test/stripe/stripe_client_test.rb | 10 ---------- 2 files changed, 1 insertion(+), 31 deletions(-) diff --git a/lib/stripe/stripe_client.rb b/lib/stripe/stripe_client.rb index 5a4069ae..4eaa6022 100644 --- a/lib/stripe/stripe_client.rb +++ b/lib/stripe/stripe_client.rb @@ -219,7 +219,7 @@ module Stripe # will throw away the result of this encoder and build its body. def encode(hash) @cache.fetch(hash) do |k| - @cache[k] = Util.encode_parameters(replace_faraday_io(hash)) + @cache[k] = Util.encode_parameters(hash) end end @@ -227,26 +227,6 @@ module Stripe def decode(_str) raise NotImplementedError, "#{self.class.name} does not implement #decode" end - - # Replaces instances of `Faraday::UploadIO` with a simple string - # representation so that they'll log a little better. Faraday won't use - # these parameters, so it's okay that we did this. - # - # Unfortunately the string representation still doesn't look very nice - # because we still URL-encode special symbols in the final value. It's - # possible we could stop doing this and just leave it to Faraday to do - # the right thing. - private def replace_faraday_io(value) - if value.is_a?(Array) - value.each_with_index { |v, i| value[i] = replace_faraday_io(v) } - elsif value.is_a?(Hash) - value.each { |k, v| value[k] = replace_faraday_io(v) } - elsif value.is_a?(Faraday::UploadIO) - "FILE:#{value.original_filename}" - else - value - end - end end def api_url(url = "", api_base = nil) diff --git a/test/stripe/stripe_client_test.rb b/test/stripe/stripe_client_test.rb index fe098c6e..8f1a7dca 100644 --- a/test/stripe/stripe_client_test.rb +++ b/test/stripe/stripe_client_test.rb @@ -794,16 +794,6 @@ module Stripe assert(!trace_payload["last_request_metrics"]["request_duration_ms"].nil?) end end - - context "FaradayStripeEncoder" do - should "replace Faraday::UploadIO instances in parameters" do - encoder = StripeClient::FaradayStripeEncoder.new - encoded = encoder.encode(foo: [ - { file: Faraday::UploadIO.new(::File.new(__FILE__), nil) }, - ]) - assert_equal "foo[0][file]=FILE%3Astripe_client_test.rb", encoded - end - end end class SystemProfilerTest < Test::Unit::TestCase