feat: swap over to websockets if initial derp exchange fails #8
Conversation
c2cd7d4 to
acb075f
Compare
acb075f to
0e3e496
Compare
31045cf to
66eac1a
Compare
|
|
||
| // Only on StatusCode < 500 in case a gateway timeout | ||
| // or network intermittency occurs! | ||
| if resp.StatusCode < 500 { |
There was a problem hiding this comment.
Is this the only status code it could be? I could see a proxy just dropping the connection with no status if it didn't like the Upgrade, or returning 400 perhaps?
There was a problem hiding this comment.
400 seems to be the standard status code if an upgrade fails, but I'm not sure we can guarantee that with all load balancers that clouds may have. That's why I chose < 500 because it'd generally mean that the server intentionally chose to not accept the WebSocket for some reason, which coderd will never do.
| c, res, err := websocket.Dial(ctx, urlStr, &websocket.DialOptions{ | ||
| Subprotocols: []string{"derp"}, | ||
| HTTPClient: &http.Client{ |
There was a problem hiding this comment.
This isn't available in js, so we might want to move this assignment out to a separate file behind build tag. Maybe a function like wsDialOptionsWithHTTPClient(opts, tlsConfig).
|
|
||
| // Only on StatusCode < 500 in case a gateway timeout | ||
| // or network intermittency occurs! | ||
| if resp.StatusCode < 500 { |
There was a problem hiding this comment.
Should we limit this to just 400 error codes or do we run into others as well?
There was a problem hiding this comment.
The honest answer is that I'm not really sure, maybe it's safe to assume only 400, but I didn't want to be presumptuous for what all proxies might do... we get the error message surfaced, so it's not impossible for us to narrow this down in the future.
cd81e90 to
d5c68ba
Compare
Many customers have load balancers that don't allow
Upgrade: derp. We still want to allow connectivity, but show a warning as to why it's failing, and what customers can do to fix it.A few of these fixes are going upstream here tailscale#7401