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

Fix OkHttp JavaNetAuthenticator with null check #177

Merged
merged 1 commit into from Jun 1, 2018

Conversation

Projects
None yet
4 participants
@eed3si9n
Member

eed3si9n commented Oct 24, 2017

Fixes sbt/sbt#3519

@eed3si9n eed3si9n requested a review from cunei Oct 24, 2017

@eed3si9n

This comment has been minimized.

Member

eed3si9n commented Oct 25, 2017

I just created a branch on sbt/sbt-standalone-build without the gigahorse=false flag - sbt/sbt-validator@b2c3daa

@dwijnand

This comment has been minimized.

Member

dwijnand commented Oct 25, 2017

I wish the "fix" was more sophisticated.. Do we have any understanding what the cause is?

@eed3si9n

This comment has been minimized.

@eed3si9n

This comment has been minimized.

Member

eed3si9n commented Oct 25, 2017

Also Dbuild is saying that a28d7f0 didn't fix.

@eed3si9n eed3si9n changed the title from Add retry around OkHttp calls to Fix OkHttp JavaNetAuthenticator with null check Oct 25, 2017

@eed3si9n

This comment has been minimized.

Member

eed3si9n commented Oct 25, 2017

oh lol. I just realized that Dbuild failure was not a failure on a28d7f0, but rather on the sbt 1.0.2 that it's using to build.

@typesafe-tools

This comment has been minimized.

typesafe-tools commented Oct 26, 2017

The validator has checked the following projects against Scala 2.12,
tested using dbuild, projects built on top of each other.

Project Reference Commit
sbt 1.x sbt/sbt@8eb5879
zinc 1.x sbt/zinc@59bc417
io 1.x sbt/io@e8c9757
librarymanagement pull/177/head 91b0085
util 1.x sbt/util@f4eadfc
website 1.x

The result is: FAILED
(restart)

@eed3si9n

This comment has been minimized.

Member

eed3si9n commented Jan 10, 2018

Why was this closed?

@dwijnand

This comment has been minimized.

Member

dwijnand commented Jan 10, 2018

It auto-closed because I deleted the 1.0.x branch. Sorry, I should remember that of this effect.

@dwijnand dwijnand reopened this Jan 10, 2018

@dwijnand dwijnand changed the base branch from 1.0.x to 1.1.x Jan 10, 2018

@denisgarci

This comment has been minimized.

denisgarci commented May 31, 2018

Any updates on this fix? This bug is blocking to update to sbt 1.1.x. Thanks!

@dwijnand

This comment has been minimized.

Member

dwijnand commented Jun 1, 2018

Closing as it's stale and failing CI.

The underlying issue is still tracked as sbt/sbt#3519 and there's a link from that to this, so this work can be recovered, revisited and resubmitted.

@dwijnand dwijnand closed this Jun 1, 2018

@eed3si9n

This comment has been minimized.

Member

eed3si9n commented Jun 1, 2018

This PR actually addresses one of the known patterns of NPE that was reported sbt/sbt#3519 (comment)
Maybe we should:

  1. Merge this PR once it passes the tests. (I don't think failures are related to this)
  2. and send the patch upstream.

@eed3si9n eed3si9n reopened this Jun 1, 2018

@typesafe-tools

This comment has been minimized.

typesafe-tools commented Jun 1, 2018

A validation involving this pull request is in progress...

@eed3si9n

This comment has been minimized.

Member

eed3si9n commented Jun 1, 2018

The stacktrace says:

[error] java.lang.NullPointerException
[error] 	at okhttp3.JavaNetAuthenticator.authenticate(JavaNetAuthenticator.java:37)
[error] 	at okhttp3.internal.http.RetryAndFollowUpInterceptor.followUpRequest(RetryAndFollowUpInterceptor.java:284)
...

okhttp code says:

    Proxy proxy = route.proxy();

What this PR adds:

+    Proxy proxy = null;
+    if (route != null) {
+      proxy = route.proxy();
+    }
@eed3si9n

This comment has been minimized.

Member

eed3si9n commented Jun 1, 2018

Travis CI is a go:

build__709_-_sbt_librarymanagement_-_travis_ci

@dwijnand dwijnand merged commit f2f1458 into sbt:1.1.x Jun 1, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@eed3si9n eed3si9n deleted the eed3si9n:wip/npe_workaround branch Jun 1, 2018

@dwijnand dwijnand added this to the 1.1.6 milestone Jun 1, 2018

@typesafe-tools

This comment has been minimized.

typesafe-tools commented Jun 1, 2018

The validator has checked the following projects against Scala 2.12,
tested using dbuild, projects built on top of each other.

Project Reference Commit
sbt 1.1.x sbt/sbt@17f5bc1
zinc 1.1.x sbt/zinc@899d7bd
io 1.1.x sbt/io@1534128
librarymanagement pull/177/head ac583b4
util 1.1.x sbt/util@d40517b
website 1.1.x

The result is: SUCCESS
(restart)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment