core: throw IOException when ProxySelector returns null or empty list

[mfperminov]

ProxySelector.select(URI) is contractually required to return a non-null,
non-empty list. Some implementations violate this, which previously caused
an opaque crash in ProxyDetectorImpl:

java.lang.IndexOutOfBoundsException: Index: 0
    at java.util.Collections$EmptyList.get
    at io.grpc.internal.ProxyDetectorImpl.detectProxy

Detect this case explicitly and throw an IOException naming the offending
ProxySelector class, so a broken implementation can be identified and
fixed by its author rather than silently worked around in every caller.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: mfperminov / name: Mikhail Perminov (ac43fae)

[mfperminov]

Hi @ejona86, would you be the right person to review this small proxy detection fix?

[ejona86]

Can we identify the broken Android deployments? I know we can't fix them, but we can document what software we are working around. The Java API says there will always be entries in the list, so both null and empty list are invalid. I'd normally suggest we fail (cleanly, not like it is now) instead of ignoring the API breakage.

[mfperminov]

Can we identify the broken Android deployments? I know we can't fix them, but we can document what software we are working around. The Java API says there will always be entries in the list, so both null and empty list are invalid. I'd normally suggest we fail (cleanly, not like it is now) instead of ignoring the API breakage.

Thanks for the context.
I unfortunately cannot provide detailed production/device information because of NDA. The only information I can share is that, in our production crash reports, more than 90% of the occurrences are on Android 14 devices, across multiple manufacturers.
Given that the Java API contract requires the returned list to be non-empty, I understand your concern about silently treating this as "no proxy".
If I understand your suggestion correctly, the preferred direction would be to fail explicitly and add a comment explaining the workaround/behavior, e.g. replace the current accidental IndexOutOfBoundsException with a clearer exception such as IllegalStateException when ProxySelector returns an empty list.
Is that the approach you would prefer for this PR?

[ejona86]

So I've looked around some, and I don't see any other reports of something like this. I'm very suspicious, as Android 14 is plenty old enough for us to have heard of this problem. It is seeming more likely at this point that maybe you have a broken ProxySelector configured somewhere in your app.

How does this sound: Add in checks for null/isEmpty(), and throw an IOException with the classname of the ProxySelector as part of the error string. I know exception strings are often discarded on Android, but this has no semantic concerns and would help anyone that accidentally writes a broken ProxySelector. Ideally the ProxySelector would get fixed, instead of all callers working around it.

If we find out this behavior is widespread we would re-evaluate the approach.

[kannanjgithub]

@mfperminov are you planning to add the debug info suggested?

[mfperminov]

@mfperminov are you planning to add the debug info suggested?

Yes, I’ll add the suggested debug info today.

[mfperminov]

@ejona86 @kannanjgithub
made an update

[kannanjgithub]

/gcbrun

[mfperminov]

The macOS Kokoro failure appears to be an infrastructure/Homebrew issue before
the build starts:

brew install --cask temurin@8 failed with
Error: undefined method 'first' for nil.

Could anyone please rerun the macOS presubmit?

[kannanjgithub]

We are yet to fix the macOS build, it happens on all PRs.