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

WebClient completes abruptly emitting null [SPR-15784] #20339

Closed
spring-issuemaster opened this issue Jul 17, 2017 · 9 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Jul 17, 2017

Abhijit Sarkar opened SPR-15784 and commented

It appears that in some situations, the WebClient completes abruptly emitting a null. Consider the following code in Kotlin that verifies whether a link is active or not:

Mono.justOrEmpty(cache.getIfPresent(path))
    .map { it as Boolean }
    .switchIfEmpty(webClient
            .head()
            .uri(link)
            .exchange()
            .retry(2L, {
                when (it) {
                    is UnknownHostException -> false
                    is IOException -> true
                    else -> false
                }
            })
            .map { it.statusCode().value() }
            .doOnNext { log.debug("Received status code: {} from: {}.", it, link) }
            .map { it < 400 }
            .doOnSuccess { if (Objects.nonNull(it)) cache.put(path, it) }
            .onErrorResume { t -> log.error("Failed to verify link: {}.", link, t); Mono.just(false) }

For URL http://www.opensource.org/licenses/cddl1.php, above code sometimes calls doOnSuccess with null; it is unacceptable to put a null in the stream.

Related SO threads:

Reactive WebClient not emitting a response

WebClient not emitting a response


Affects: 5.0 RC2

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 18, 2017

Rossen Stoyanchev commented

Simply running the code as is I get 200 back. Not very exciting. It sounds like it this occurs if the exchange completes empty. How did you get the condition to occur? Also could you insert .log() after exchange and after retry to see what signals flow?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 18, 2017

Rossen Stoyanchev commented

I'm scheduling for RC3 since in the very least we can protect the WebClient exchange method from ever completing empty. That said there is likely a Reactor Netty fix as mentioned by the Brian Clozel on the fist SO thread. Brian will comment with link here when that ticket is created.

As a side note, the null in doOnSuccess is expected behavior. See javadoc of Mono#doOnSuccess for when the Mono has completed empty. The doOnSuccess method is a notification and not part of the reactive signals. What's actually flowing through is a completion signal.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 18, 2017

Brian Clozel commented

I've linked this issue to the reactor-netty one I've just created.

Abhijit Sarkar, could you try the workaround I'm suggesting in my SO answer? If that works, that would confirm that this is indeed related to the reactor-netty issue.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 18, 2017

Rossen Stoyanchev commented

For the time being I've added defensive checks to ensure there is never completion with neither a ClientResponse nor an error signal. This includes a second (temporary) check at the Reactor Netty connector level that provides a direct reference to the Reactor Netty issue and possible workaround (that still needs to be confirmed).

This is all we can do from our end for now so I am going to resolve the ticket but Abhijit Sarkar please do comment and we'll also continue to update it with progress from the Reactor Netty side.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 19, 2017

Abhijit Sarkar commented

@Rossen Stoyanchev, what is the change in behavior due to the checks you added? Currently the publisher completes silently; if you'll be emitting an error instead, that'll be unexpected because closing the connection from the server side is a perfectly valid scenario.

P.S.: Seems that I can't reference by name using @username but you can. I'm reopening the issue to bring my comment to your attention because without a name reference, I'm not sure if closed issues pop up on your radar.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 19, 2017

Rossen Stoyanchev commented

If you see the Reactor Netty ticket Brian opened (referenced above), the suspected issue is a race condition in the Reactor Netty HttpClient, i.e. not a normal closing of the connection from the server side. Whatever the root case, the exchange() should always yield a ClientResponse or an error, never complete empty right? Currently we're not getting a ClientHttpResponse from the Reactor Netty HttpClient so it's not an issue we can fix in the Spring Framework.

BTW how do you get the condition to occur, performance test? Do you have any way to confirm Brian's workaround suggestion to turn off connection pooling from the first SO link?

(Yes I do get notifications on this ticket even without mentions)

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 19, 2017

Abhijit Sarkar commented

how do you get the condition to occur

Simply in my integration test, using the URL in the ticket. Brian was also able to reproduce the issue using a sample project that he attached with the Netty ticket. This is a sample app that I'm playing with, no performance testing involved. Which is why I'm having a hard time accepting the race condition theory; there's only one thread running. That said, see my answer to your question below.

Do you have any way to confirm Brian's workaround suggestion

Yes, I tried with disablePool, and it seems to be working. However, it does make the exchange slow.

The code is here

The test is here

Code to disable pool is here

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 19, 2017

Brian Clozel commented

For this to happen, you only need a server that closes the TCP connection; since you're running your tests against third party servers (probably a bad idea anyway), maybe with reverse proxies and CDNs - they might close the connection.

Now reactor is a reactive runtime, so there are multiple threads involved; in fact, multiple threads might be involved for a single HTTP request.

Since the disable pool option "fixes" the issue, this points to the issue I've opened against reactor-netty. The race condition doesn't happen in your code but probably in the code managing the client connection pool.

With the workaround Rossen committed and the confirmation of this issue on reactor-netty, I'm in favor of closing this issue and pointing to reactor-netty#138.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 19, 2017

Rossen Stoyanchev commented

Resolving again, but not closing, so you can continue to comment. We do see those regardless of the status.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.