Skip to content

Commit

Permalink
Remove attempt to nicen UploadIO in logs
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
brandur committed Feb 11, 2019
1 parent e0438d2 commit cf6d79a
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 31 deletions.
22 changes: 1 addition & 21 deletions lib/stripe/stripe_client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -219,34 +219,14 @@ def initialize
# 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

# We should never need to do this so it's not implemented.
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)
Expand Down
10 changes: 0 additions & 10 deletions test/stripe/stripe_client_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -794,16 +794,6 @@ class StripeClientTest < Test::Unit::TestCase
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
Expand Down

0 comments on commit cf6d79a

Please sign in to comment.