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.

Sign up for GitHub

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

Jump to bottom

Fix route failure recovery for HTTP/2 #578

Closed
swankjesse opened this issue Mar 1, 2014 · 1 comment · Fixed by #3059
Closed

Fix route failure recovery for HTTP/2 #578

swankjesse opened this issue Mar 1, 2014 · 1 comment · Fixed by #3059
Labels
bug Bug in existing code
Milestone
3.6

Comments

@swankjesse
Copy link
Member

As awkwardly demonstrated in HttpOverSpdyTest.readResponseHeaderTimeout, we attempt to recover from timeouts on SPDY connections. But these timeouts are not connectivity problems; they're application-layer timeouts.

The test passes, even though we only ever establish a single SPDY connection to the server. This is two mistakes canceling each other out.

When we have an application-layer SPDY timeout, we shouldn't treat it as a connectivity problem and retry. When we have a socket-layer SPDY timeout, we should treat it as a connectivity problem.
@swankjesse swankjesse added this to the 3.0 milestone Mar 1, 2014
@swankjesse swankjesse added the bug label Mar 1, 2014
swankjesse added a commit that referenced this issue Mar 1, 2014
@swankjesse
Ignore a broken test for SPDY failure recovery.
8b0401e 
#578
@swankjesse swankjesse mentioned this issue Mar 1, 2014
Merged
@swankjesse
Copy link
Member Author

I investigated this. The trick is that sometimes there is a legitimate connectivity problem. The solution might be to use pings or something when a connection appears to be dead.

@swankjesse swankjesse modified the milestones: 3.1, 3.0 Dec 31, 2015
@swankjesse swankjesse modified the milestones: 3.4, 3.3 May 8, 2016
@swankjesse swankjesse modified the milestones: 3.6, 3.5 Oct 16, 2016
swankjesse added a commit that referenced this issue Dec 24, 2016
@swankjesse
Confirm that HTTP/2 failure recovery is decent.
219f286 
This was fixed recently but we didn't update the corresponding test case. This
updates the test and adds a few more to confirm that HTTP/2 failure recovery
works as it should.

Closes: #578
@swankjesse swankjesse mentioned this issue Dec 24, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Bug in existing code
Projects
None yet
Milestone
3.6
Development

Successfully merging a pull request may close this issue.

Confirm that HTTP/2 failure recovery is decent. square/okhttp
1 participant
@swankjesse