feat: automatically use websockets if DERP upgrade is unavailable #6381
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This might be our biggest hangup for deployments at the moment... Load balancers by default do not support the DERP protocol, so many of our prospects and customers run into failing workspace connections. This automatically swaps to use WebSockets, and reports the reason to coderd. In a future contribution, a warning will appear by the agent if it was forced to use WebSockets instead of DERP.
7cc624e to
589ead1
Compare
ad23191 to
44aa875
Compare
|
@mafredri I'm going to wait to merge this until you approve... I've brought too many race conditions into this code to risk introducing more 😅😅😅 |
|
Resolves #6375 |
deansheather
reviewed
Mar 1, 2023
mafredri
approved these changes
Mar 1, 2023
| // speak WebSockets (they still assumed DERP's binary framing). So to distinguish | ||
| // clients that actually want WebSockets, look for an explicit "derp" subprotocol. | ||
| if up != "websocket" || !strings.Contains(r.Header.Get("Sec-Websocket-Protocol"), "derp") { | ||
| base.ServeHTTP(w, r) |
There was a problem hiding this comment.
Should we log here to better understand why it may not be working? (E.g. future bugs.)
There was a problem hiding this comment.
I don't think so because this could be spammy.
91fbf09 to
b830fad
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This might be our biggest hangup for deployments at the moment...
Load balancers do not support the DERP protocol by default (and many do not at all), so many of our prospects and customers run into failing workspace connections. After the first DERP connection HTTP response with a non
http.StatusSwitchingProtocolsresponse code (and <500), we will begin using WebSockets instead of DERP. It will never attempt to switch back for now.In a future contribution, a warning will appear by the agent if it was forced to use WebSockets instead of DERP.
Depends on coder/tailscale#8
Related #6372