Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stripe-ruby V5 #815

Merged
merged 18 commits into from
Aug 20, 2019
Merged

stripe-ruby V5 #815

merged 18 commits into from
Aug 20, 2019

Commits on Aug 16, 2019

  1. Convert library to use built-in Net::HTTP

    Moves the library off of Faraday and over onto the standard library's
    built-in `Net::HTTP` module. The upside of the transition is that we
    break away from a few dependencies that have caused us a fair bit of
    trouble in the past, the downside is that we need more of our own code
    to do things (although surprisingly, not that much more).
    
    The biggest new pieces are:
    
    * `ConnectionManager`: A per-thread class that manages a connection to
      each Stripe infrastructure URL (like `api.stripe.com`,
      `connect.stripe.com`, etc.) so that we can reuse them between
      requests. It's also responsible for setting up and configuring new
      `Net::HTTP` connections, which is a little more heavyweight
      code-wise compared to other libraries. All of this could have lived in
      `StripeClient`, but I extracted it because that class has gotten so
      big.
    
    * `MultipartEncoder`: A class that does multipart form encoding for file
      uploads. Unfortunately, Ruby doesn't bundle anything like this. I
      built this by referencing the Go implementation because the original
      RFC is not very detailed or well-written. I also made sure that it was
      behaving similarly to our other custom implementations like
      stripe-node, and that it can really upload a file outside the test
      suite.
    
    There's some risk here in that it's easy to miss something across one of
    these big transitions. I've tried to test out various error cases
    through tests, but also by leaving scripts running as I terminate my
    network connection and bring it back. That said, we'd certainly release
    on a major version bump because some of the interface (like setting
    `Stripe.default_client`) changes.
    brandur authored and ob-stripe committed Aug 16, 2019
    Configuration menu
    Copy the full SHA
    68d202b View commit details
    Browse the repository at this point in the history
  2. Drop support for old versions of Ruby

    Drops support for Ruby 2.1 (EOL March 31, 2017) and 2.2 (EOL March 31,
    2018). They're removed from `.travis.yml` and the gemspec and RuboCop
    configuration have also been updated to the new lower bound.
    
    Most of the diff here are minor updates to styling as required by
    RuboCop:
    
    * String literals are frozen by default, so the `.freeze` we had
      everywhere is now considered redundant.
    
    * We can now use Ruby 1.9 style hash syntax with string keys like `{
      "foo": "bar" }`.
    
    * Converted a few heredocs over to use squiggly (leading whitespace
      removed) syntax.
    
    As discussed in Slack, I didn't drop support for Ruby 2.3 (EOL March 31,
    2019) as we still have quite a few users on it. As far as I know
    dropping it doesn't get us access to any major syntax improvements or
    anything, so it's probably not a big deal.
    brandur authored and ob-stripe committed Aug 16, 2019
    Configuration menu
    Copy the full SHA
    15538b0 View commit details
    Browse the repository at this point in the history
  3. Make CardError's code parameter named instead of positional (#816)

    Makes the `code` parameter on `CardError` named instead of positional.
    This makes it more consistent with the rest of the constructor's
    parameters and makes instantiating `CardError` from `StripeClient`
    cleaner.
    
    This is a minor breaking change so we're aiming to release it for the
    next major version of stripe-ruby.
    brandur authored and ob-stripe committed Aug 16, 2019
    Configuration menu
    Copy the full SHA
    4455556 View commit details
    Browse the repository at this point in the history
  4. Configuration menu
    Copy the full SHA
    f4cd69b View commit details
    Browse the repository at this point in the history
  5. Configuration menu
    Copy the full SHA
    cf45ea7 View commit details
    Browse the repository at this point in the history
  6. Configuration menu
    Copy the full SHA
    53c96e6 View commit details
    Browse the repository at this point in the history
  7. Configuration menu
    Copy the full SHA
    fd004e5 View commit details
    Browse the repository at this point in the history
  8. Configuration menu
    Copy the full SHA
    b87c5a5 View commit details
    Browse the repository at this point in the history
  9. Configuration menu
    Copy the full SHA
    56ea5f4 View commit details
    Browse the repository at this point in the history
  10. Configuration menu
    Copy the full SHA
    e840ce8 View commit details
    Browse the repository at this point in the history
  11. Tweak retry logic to be a little more like stripe-node (#828)

    Tweaks the retry logic to be a little more like stripe-node's. In
    particular, we also retry under these conditions:
    
    * If we receive a 500 on a non-`POST` request.
    * If we receive a 503.
    
    I made it slightly different from stripe-node which checks for a 500
    with `>= 500`. I don't really like that -- if we want to retry specific
    status codes we should be explicit about it.
    
    We're actively re-examining ways on how to make it easier for clients to
    figure out when to retry right now, but I figure V5 is a good time to
    tweak this because the modifications change the method signature of
    `should_retry?` slightly, and it's technically a public method.
    brandur authored and brandur-stripe committed Aug 16, 2019
    Configuration menu
    Copy the full SHA
    084276b View commit details
    Browse the repository at this point in the history

Commits on Aug 17, 2019

  1. Fix inverted sign for 500 retries (#830)

    I messed up in #828 by (1) accidentally flipping the comparison against
    `:post` when checking whether to retry on 500, and (2) forgetting to
    write new tests for the condition, which is how (1) got through.
    
    This patch fixes both those problems.
    brandur authored and brandur-stripe committed Aug 17, 2019
    Configuration menu
    Copy the full SHA
    4d52345 View commit details
    Browse the repository at this point in the history
  2. Remove a few more very old deprecated methods (#831)

    I noticed that we had a couple of other deprecated methods on `Stripe`
    and `StripeObject` that have been around for a long time. May as well
    get rid of them too -- luckily they were using `Gem::Deprecate` so
    they've been producing annoying deprecated warnings for quite a while
    now.
    brandur authored and brandur-stripe committed Aug 17, 2019
    Configuration menu
    Copy the full SHA
    0b58fb8 View commit details
    Browse the repository at this point in the history

Commits on Aug 19, 2019

  1. Configuration menu
    Copy the full SHA
    a1249af View commit details
    Browse the repository at this point in the history
  2. Reset connections when connection-changing configuration changes (#829)

    Adds a few basic features around connection and connection manager
    management:
    
    * `clear` on connection manager, which calls `finish` on each active
      connection and then disposes of it.
    
    * A centralized cross-thread tracking system for connection managers in
      `StripeClient` and `clear_all_connection_managers` which clears all
      known connection managers across all threads in a thread-safe way.
    
    The addition of these allow us to modify the implementation of some of
    our configuration on `Stripe` so that it can reset all currently open
    connections when its value changes.
    
    This fixes a currently problem with the library whereby certain
    configuration must be set before the first request or it remains fixed
    on any open connections. For example, if `Stripe.proxy` is set after a
    request is made from the library, it has no effect because the proxy
    must have been set when the connection was originally being initialized.
    
    The impetus for getting this out is that I noticed that we will need
    this internally in a few places when we're upgrading to stripe-ruby V5.
    Those spots used to be able to hack around the unavailability of this
    feature by just accessing the Faraday connection directly and resetting
    state on it, but in V5 `StripeClient#conn` is gone, and that's no longer
    possible.
    brandur authored and brandur-stripe committed Aug 19, 2019
    Configuration menu
    Copy the full SHA
    a9a7af0 View commit details
    Browse the repository at this point in the history
  3. Minor cleanup in StripeClient (#832)

    I ended up having to relax the maximum method line length in a few
    previous PRs, so I wanted to try one more cleanup pass in
    `execute_request` to see if I could get it back at all.
    
    The answer was "not by much" (without reducing clarity), but I found a
    few places that could be tweaked. Unfortunately, ~50 lines is probably
    the "right" length for this method in that you _could_ extract it
    further, but you'd end up passing huge amounts of state all over the
    place in method parameters, and it really wouldn't look that good.
    brandur authored and brandur-stripe committed Aug 19, 2019
    Configuration menu
    Copy the full SHA
    0abfc55 View commit details
    Browse the repository at this point in the history

Commits on Aug 20, 2019

  1. Do better bookkeeping when tracking state in Thread.current (#833)

    This is largely just another cleanup patch, but does a couple main
    things:
    
    * Hoists the `last_response` value into thread state. This is a very
      minor nicety, but effectively makes `StripeClient` fully thread-safe,
      which seems like a minor nicety. Two calls to `#request` to the same
      `StripeObject` can now be executed on two different threads and their
      results won't interfere with each other.
    
    * Moves state off one-off `Thread.current` keys and into a single one
      for the whole client which stores a new simple type of record called
      `ThreadContext`. Again, this doesn't change much, but adds some minor
      type safety and lets us document each field we expect to have in a
      thread's context.
    brandur authored and brandur-stripe committed Aug 20, 2019
    Configuration menu
    Copy the full SHA
    95ac3de View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    ff63ef4 View commit details
    Browse the repository at this point in the history