Skip to content

Commit

Permalink
Improve coverage, revert zero duration change
Browse files Browse the repository at this point in the history
  • Loading branch information
echlebek committed Feb 14, 2024
1 parent f11788b commit 341966d
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 16 deletions.
6 changes: 1 addition & 5 deletions client/wsclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package client
import (
"context"
"errors"
"fmt"
"net/http"
"net/url"
"sync"
Expand Down Expand Up @@ -137,7 +136,7 @@ func (c *wsClient) tryConnectOnce(ctx context.Context) (err error, retryAfter sh
// very liberal handling of 3xx that largely ignores HTTP semantics
redirect, err := resp.Location()
if err != nil {
fmt.Println("error in location", err)
c.common.Callbacks.OnConnectFailed(ctx, err)
c.common.Logger.Errorf(ctx, "%d redirect, but no valid location: %s", resp.StatusCode, err)
return err, duration
}
Expand All @@ -151,9 +150,6 @@ func (c *wsClient) tryConnectOnce(ctx context.Context) (err error, retryAfter sh
// Set the URL to the redirect, so that it connects to it on the
// next cycle.
c.url = redirect

// don't delay in following the redirect
duration = sharedinternal.ZeroDuration
} else {
c.common.Logger.Errorf(ctx, "Server responded with status=%v", resp.Status)
}
Expand Down
28 changes: 23 additions & 5 deletions client/wsclient_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,25 +181,37 @@ func TestVerifyWSCompress(t *testing.T) {
}
}

func redirectServer(to string) *httptest.Server {
func redirectServer(to string, status int) *httptest.Server {
return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
http.Redirect(w, req, to, http.StatusSeeOther)
}))
}

func errServer() *httptest.Server {
return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
w.WriteHeader(302)
}))
}

func TestRedirectWS(t *testing.T) {
redirectee := internal.StartMockServer(t)
tests := []struct {
Name string
Redirector *httptest.Server
ExpError bool
}{
{
Name: "redirect ws scheme",
Redirector: redirectServer("ws://" + redirectee.Endpoint),
Redirector: redirectServer("ws://"+redirectee.Endpoint, 302),
},
{
Name: "redirect http scheme",
Redirector: redirectServer("http://" + redirectee.Endpoint),
Redirector: redirectServer("http://"+redirectee.Endpoint, 302),
},
{
Name: "missing location header",
Redirector: errServer(),
ExpError: true,
},
}

Expand Down Expand Up @@ -233,8 +245,14 @@ func TestRedirectWS(t *testing.T) {
startClient(t, settings, client)

// Wait for connection to be established.
eventually(t, func() bool { return conn.Load() != nil })
assert.True(t, connectErr.Load() == nil)
eventually(t, func() bool { return conn.Load() != nil || connectErr.Load() != nil })
if test.ExpError {
if connectErr.Load() == nil {
t.Error("expected non-nil error")
}
} else {
assert.True(t, connectErr.Load() == nil)
}

// Stop the client.
err = client.Stop(context.Background())
Expand Down
6 changes: 0 additions & 6 deletions internal/retryafter.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,6 @@ type OptionalDuration struct {
Defined bool
}

// ZeroDuration represents a zero length duration that is defined.
var ZeroDuration = OptionalDuration{
Duration: 0,
Defined: true,
}

func parseDelaySeconds(s string) (time.Duration, error) {
n, err := strconv.Atoi(s)

Expand Down

0 comments on commit 341966d

Please sign in to comment.