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

Disable HTTP/2 by default #903

Merged
merged 1 commit into from
Jul 25, 2019
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 38 additions & 1 deletion stripe.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package stripe

import (
"bytes"
"crypto/tls"
"encoding/json"
"errors"
"fmt"
Expand Down Expand Up @@ -944,7 +945,43 @@ var appInfo *AppInfo
var backends Backends
var encodedStripeUserAgent string
var encodedUserAgent string
var httpClient = &http.Client{Timeout: defaultHTTPTimeout}

// The default HTTP client used for communication with any of Stripe's
// backends.
//
// Can be overridden with the function `SetHTTPClient` or by setting the
// `HTTPClient` value when using `BackendConfig`.
var httpClient = &http.Client{
Timeout: defaultHTTPTimeout,

// There is a bug in Go's HTTP/2 implementation that occasionally causes it
// to send an empty body when it receives a `GOAWAY` message from a server:
//
// https://github.com/golang/go/issues/32441
//
// This is particularly problematic for this library because the empty body
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great comment; thanks for capturing all the context in here. It's my sincere hope that we can re-enable H/2 support soon, when that Go issue gets addressed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! And yep, totally +1.

// results in no parameters being sent, which usually results in a 400,
// which is a status code expressly not covered by retry logic.
//
// The bug seems to be somewhat tricky to fix and hasn't seen any traction
// lately, so for now we're mitigating by disabling HTTP/2 in stripe-go by
// default. Users who like to live dangerously can still re-enable it by
// specifying a custom HTTP client. When the bug above is fixed, we can
// turn it back on.
//
// The particular methodology here for disabling HTTP/2 is a little
// confusing at first glance, but is recommended by the `net/http`
// documentation ("Programs that must disable HTTP/2 can do so by setting
// Transport.TLSNextProto (for clients) ... to a non-nil, empty map.")
//
// Note that the test suite still uses HTTP/2 to run as it specifies its
// own HTTP client with it enabled. See `testing/testing.go`.
//
// (Written 2019/07/24.)
Transport: &http.Transport{
TLSNextProto: make(map[string]func(string, *tls.Conn) http.RoundTripper),
},
}

//
// Private functions
Expand Down