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

Cannot obtain braintreee configuration from drop in #155

Closed
consp1racy opened this issue Jul 13, 2017 · 13 comments
Closed

Cannot obtain braintreee configuration from drop in #155

consp1racy opened this issue Jul 13, 2017 · 13 comments

Comments

@consp1racy
Copy link

consp1racy commented Jul 13, 2017

General information

  • SDK/Library version: api 2.5.4, drop-in 3.0.8
  • Environment: Production
  • Android Version and Device: Any

Issue description

Some clients cannot obtain configuration (using drop in ui). When deleted from braintree console the same clients that could obtain it can still obtain it (two people on our dev team), those who couldn't still can't (one tema member, one customer and supposedly 40 other customers so far).

Error while obtaining payment information. 
com.braintreepayments.api.exceptions.ConfigurationException: Request for configuration has failed: <html><body><h1>400 Bad request</h1>
Your browser sent an invalid request.
</body></html>

. Future requests will retry up to 3 times
    at com.braintreepayments.api.BraintreeFragment$10.onResponse(BraintreeFragment.java:647)
    at com.braintreepayments.api.BraintreeFragment$10.onResponse(BraintreeFragment.java:643)
    at com.braintreepayments.api.ConfigurationManager$1.failure(ConfigurationManager.java:78)
    at com.braintreepayments.api.internal.HttpClient$4.run(HttpClient.java:299)
    at android.os.Handler.handleCallback(Handler.java:743)
    at android.os.Handler.dispatchMessage(Handler.java:95)
    at android.os.Looper.loop(Looper.java:150)
    at android.app.ActivityThread.main(ActivityThread.java:5621)
    at java.lang.reflect.Method.invoke(Native Method)
    at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:794)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:684)
 Caused by: com.braintreepayments.api.exceptions.UnexpectedException: <html><body><h1>400 Bad request</h1>
Your browser sent an invalid request.
</body></html>


    at com.braintreepayments.api.internal.HttpClient.parseResponse(HttpClient.java:274)
    at com.braintreepayments.api.internal.BraintreeHttpClient.parseResponse(BraintreeHttpClient.java:138)
    at com.braintreepayments.api.internal.HttpClient$1.run(HttpClient.java:155)
    at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:423)
    at java.util.concurrent.FutureTask.run(FutureTask.java:237)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1113)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:588)
    at java.lang.Thread.run(Thread.java:833)
@lkorth
Copy link
Member

lkorth commented Jul 17, 2017

To clarify:

When deleted from braintree console the same clients that could obtain it can still obtain it (two people on our dev team), those who couldn't still can't (one tema member, one customer and supposedly 40 other customers so far).

Are you deleting a tokenization key from the control panel and then attempting to get configuration with the deleted tokenization key afterwards? Due to caching, configuration will continue to be available for a short period of time after the tokenization key is deleted.

@consp1racy
Copy link
Author

Hotfix part 1

In Core/com.braintreepayments.api.internal.HttpClient move

connection.setRequestProperty("Content-Type", "application/json");

from #init to #post.

GET requests don't have body and therefore Content-Type.
Braintree server should probably ignore this on GET requests in the future.

Hotfix part 2

In Core/com.braintreepayments.api.internal.HttpClient delete

connection.setRequestProperty("Accept-Encoding", "gzip");

from #init.

I have no idea why this was necessary. 🤔

Misc

When testing in Advanced REST Client removing any one of these header helped. On an actual device only removing both headers helped. No clue why disabling gzip affected anything, the original error came from server not during parsing.

I'm able to use Drop In now (by modifying the Core library), but it's still a workaround rather than a fix.

@consp1racy
Copy link
Author

Are you deleting a tokenization key from the control panel

Yes

and then attempting to get configuration with the deleted tokenization key afterwards?

No, we cleared app data on the device, discarded any locally prepared client token, and obtained a new client token through our server which is in thia context just a relay to Braintree (no caching on our server).

@lkorth
Copy link
Member

lkorth commented Jul 21, 2017

Deleting a tokenization key will not affect clients that are using client tokens, for more information about how tokenization keys and client tokens differ see the client authorization docs.

Are there specific devices that this happens on? This code has been present in version 2 of the SDK for nearly 2 years now and we have never encountered an issue like this or had one reported. Since we cannot reproduce it, it is difficult to determine the fix.

  1. You're right Content-Type should not be set for GET requests even if it currently works. I've pushed a fix for this and it will be available in 2.5.5-SNAPSHOT as soon as the travis build finishes.
  2. Gzip is not something that should be disabled, the reduction in response size for JSON is significant.

@consp1racy
Copy link
Author

Here's what I'll do now:

  1. Test the snapshot,
  2. Swap the http client for okhttp, see if that helps.

I totally get that gzip is something we want, I'm also confused why disabling it helped considering the issue is specific to some Braintree clients and not a device or http client.

Is switching to okhttp3 something you'd be interested in via pull request (if it works)?

@lkorth
Copy link
Member

lkorth commented Jul 24, 2017

The HttpClient class that the SDK uses is just a thin convenience layer on top of HttpUrlConnection, as of Android 4.4 Android use OkHttp for its internal HttpUrlConnection implementation.

Adding a dependency on OkHttp to fix this issue is not something we want to do, it adds additional size to the SDK and causes issues for anyone using older versions of OkHttp.

Do you have any specific versions or devices that you've encountered this issue on?

@consp1racy
Copy link
Author

consp1racy commented Jul 24, 2017 via email

@consp1racy
Copy link
Author

consp1racy commented Jul 25, 2017

TL;DR

2.5.5-SNAPSHOT added Accept: application/json header to the request. After removing it, the snapshot started working.

Which essentially translates to gzip magically starting to work. So it must've come from a change on server.

Original post

Ok, so I got more info.

  1. 2.5.5-SNAPSHOT still failed.
  2. Surprisingly switching to OkHttp3 worked even with gzip!

OkHttp operates at two levels: application and network, and you can attach loggers at each of them. So I observed what headers are set vs. what headers are actually sent.

These are headers I fed to OkHttp at the application level:

User-Agent: braintree/android/2.5.5-SNAPSHOT
Accept-Language: cs

and that translated to these headers which were actually wired over to Braintree server:

User-Agent: braintree/android/2.5.5-SNAPSHOT
Accept-Language: cs
Host: api.braintreegateway.com
Connection: Keep-Alive
Accept-Encoding: gzip

These two lines are the only technical difference between 2.5.4 and the OkHttp3 fork:

Host: api.braintreegateway.com
Connection: Keep-Alive

And both are irrelevant.

@lkorth
Copy link
Member

lkorth commented Jul 26, 2017

There has been no change in response handling on the server.

Given that the Accept header is not strictly necessary to include in these requests and that we have been unable to reproduce this issue on any device or emulator, I've removed the additional header.

You'll be able to use 2.5.5-SNAPSHOT without the header as soon as this travis build completes.

@consp1racy
Copy link
Author

Thanks, it works for now. I don't like the idea we dismiss this without finding the cause. Is there a way I can make one of those broken calls in a controlled fashion, so you can intercept the call on server and figure out why it's responding with a 400 code?

@lkorth
Copy link
Member

lkorth commented Aug 1, 2017

We're working on several changes to the service that handles these requests so it's probably best to wait until that's done before testing it further with the accept header.

@sdcoffey
Copy link

Hey @consp1racy,

As a quick update, we've completed the changes to the server that handles configuration requests as @lkorth mentioned above. If you'd like to retest this request with the Accept header, you may do so now. Keep us updated and let us know if there's anything else we can do here.

@crookedneighbor
Copy link
Contributor

Looks like this has been resolved, so I'm going to close it now. Feel free to comment here if that is not the case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants