braintree / braintree_dotnet

Braintree .NET library
https://developer.paypal.com/braintree/docs/start/overview
MIT License
136 stars 73 forks source link

Unable to set `Configuration` property after instantiating since 5.0.0 #122

Open jebax opened 2 years ago

jebax commented 2 years ago

General information

Issue description

Hi,

Our previous method of instantiating BraintreeGateway was to do something like this:

new BraintreeGateway()
{
  Configuration = new CustomBraintreeConfiguration()
}

Where CustomBraintreeConfiguration inherits from Braintree.Configuration.

Since 5.0.0, instantiating BraintreeGateway like this causes all HTTP calls to break with a NullReferenceException (from trying to access Environment.GatewayURL here).

I can see in the 5.0.0 commit that many classes now use the static property gateway.Service instead of instantiating a new service using gateway.Configuration. Since gateway.Service is assigned during the BraintreeGateway constructor, setting Configuration on the gateway after instantiation will always mean Environment is null.

Spent quite a while debugging this, and I think more people could run into this issue unless you either make the Configuration read-only or throw a more descriptive error. This breaking change also wasn't in the 5.0.0 changelog or the v5 migration guide.

Thanks!

hollabaq86 commented 2 years ago

👋 @jebax thanks for the issue, and apologies that our CHANGELOG wasn't more clear. We moved to an entirely static HTTPClient in v5 which is you're encountering issues, now. I just want to confirm, you're not encountering this issue when creating a Configuration class object outside of constructing a BraintreeGateway?

We have another issue open that's related (#123), so we'll be taking another look at gateway and config construction in future versions for sure.

jebax commented 2 years ago

👋 @jebax thanks for the issue, and apologies that our CHANGELOG wasn't more clear. We moved to an entirely static HTTPClient in v5 which is you're encountering issues, now. I just want to confirm, you're not encountering this issue when creating a Configuration class object outside of constructing a BraintreeGateway?

We have another issue open that's related (#123), so we'll be taking another look at gateway and config construction in future versions for sure.

Thanks for the reply.

Yeah there's no issues now that I'm passing a configuration object into the constructor. Sounds like it's just an issue with the changelog then, IMO it's pretty hard to know that a static HTTP client is going to affect the way you instantiate the gateway without looking at the source code!

hollabaq86 commented 2 years ago

Sounds like it's just an issue with the changelog then

💯 We can make this more clear in the CHANGELOG and our migration guides, too. I'll keep this issue open until we have that fixed up.

hollabaq86 commented 2 years ago

for internal notekeeping, ticket 1615