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

Backport PR-4489 to 3.12.x branch #4679

Closed
khltrifork opened this issue Mar 3, 2019 · 9 comments
Closed

Backport PR-4489 to 3.12.x branch #4679

khltrifork opened this issue Mar 3, 2019 · 9 comments
Labels
enhancement Feature not a bug
Milestone

Comments

@khltrifork
Copy link

I'm currently maintaining an app that uses 3.12.x version of the OkHttp library.
I am not able to upgrade to 3.13.x as i have to support API 16+ and 3.13.x only supports API 21+.

By tracking crashes in production, i've had a lot of users run into IndexOutOfBoundsException in RealConnection.java. Other users have run into this issue as well, described here:
#4427

My app calls several different endpoints on several different backends, so i don't know which one is causing issues (I can't reproduce the issue locally).
I can't get any details as to why this is happening because i only get an IndexOutOfBoundsException, and not a SSLPeerUnverifiedException (which would contain the host name details that i need).

Would it be possible to backport #4489 to the 3.12.x branch?
It seems quite trivial, but i don't know the full scope of this or how your stance is on backporting non-critical elements to LTS-branches (even if it is a minor thing like this).

@khltrifork khltrifork added the enhancement Feature not a bug label Mar 3, 2019
@swankjesse
Copy link
Collaborator

It's highly unlikely that that exact issue is the cause of your problem; that requires an anonymous cipher suite to be configured.

@khltrifork
Copy link
Author

As i have no idea what that is, you're most likely right.

But i still get reports of this crash, with the exact same stacktrace as in #4427
So i still end up reaching that code, and running into the issue.
That's why i thought backport could potentially shed some light on my issues.

If i knew what backend/endpoint was causing this, could i potentially check server certificates and see if anything is set up incorrectly? Or is this an issue that is only caused by on-device issues that are basically out of my hands?

The issue is not only happening on older device, but also quite new ones (Samsung S9, OnePlus 6, etc), so i can't simply dismiss it and say "old devices suck"

@swankjesse
Copy link
Collaborator

Any idea which URLs trigger this? Maybe captive hotspots.

@swankjesse swankjesse added this to the 3.12.2 milestone Mar 4, 2019
@khltrifork
Copy link
Author

No, sadly i don't have any idea on which URLs trigger this.
My app calls many different URLs, so it could potentially be any of them.

I have no debug information from stack traces, and i can't replicate it locally (neither can any of my colleagues).

If/when this gets backported, i will have access to stacktraces, which will contain the host name of the culprit.

@amirlivneh
Copy link
Collaborator

Maybe you can catch this exception when you call Call.execute(), and in the catch block print the URL and rethrow the exception?

@khltrifork
Copy link
Author

That won't help from what i can see?

If i simple try/catch the exceptions, then i will get an IndexOutOfBoundsException which doesn't contain any usable information.
The IndexOutOfBoundsException is thrown 1 single line before the "real" exception is thrown, so it never reaches the "real" exception. (If you look at 3.12.x branch that is, it's changed in 3.13.x because of the PR)

The PR #4489 fixes that, allowing the real exception to be thrown, with the URL that is causing issues.

@amirlivneh
Copy link
Collaborator

Don't you have access to the original Request object when you catch the exception, with the URL in it?

@khltrifork
Copy link
Author

All my API calls are called used RxJava, which uses Retrofit, which in turn uses OkHttp.

I never manually create/use Request objects.

Since i can't reproduce it locally, and would basically have to test this by pushing it into production, i'm very cautious when changing error handling.

@amirlivneh
Copy link
Collaborator

You can try registering an EventListener that logs the URL with the OkHttpClient instance used by Retrofit.

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

No branches or pull requests

3 participants