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

Update isHealthy check to be safer #8503

Closed
wants to merge 1 commit into from

Conversation

seljabali
Copy link

Fixes #8447

@prbprbprb
Copy link

But maybe masks an underlying bug? Those values shouldn't be null at this point.

@seljabali
Copy link
Author

But maybe masks an underlying bug? Those values shouldn't be null at this point.

Question is: Should isHealthy() return false or throw an exception when these values are null?

At face value, it should return false for this method, however throw an exception elsewhere when these values are non-null.

@prbprbprb
Copy link

My reading of the code is that these are preconditions, hence the use of !! in Kotlin whereas in Java we would use something like Objects.requireNotNull(). Because if the preconditions aren't met, something has already failed previously.

Caveat: I'm not an okhttp expert but happened to be looking at a similar exception in okhttp2 earlier today.

@yschimke
Copy link
Collaborator

yschimke commented Jan 4, 2025

Shouldn't be possible after https://github.com/square/okhttp/pull/8514/files, or at least this can't be applied.

That said, it's possible I just moved a logic bug up higher, but this fix isn't the right one anymore.

@yschimke yschimke closed this Jan 4, 2025
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

Successfully merging this pull request may close these issues.

NullPointerException in okhttp3.internal.connection.RealConnection.isHealthy()
3 participants