Skip to content

Commit

Permalink
Store retry errors in the wsclient
Browse files Browse the repository at this point in the history
  • Loading branch information
echlebek committed Feb 21, 2024
1 parent 341966d commit f6cc5df
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 3 deletions.
7 changes: 6 additions & 1 deletion client/wsclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"net/http"
"net/url"
"sync"
"sync/atomic"
"time"

"github.com/cenkalti/backoff/v4"
Expand Down Expand Up @@ -35,6 +36,10 @@ type wsClient struct {

// The sender is responsible for sending portion of the OpAMP protocol.
sender *internal.WSSender

// last non-nil internal error that was encountered in the conn retry loop,
// currently used only for testing.
lastInternalErr atomic.Pointer[error]
}

// NewWebSocket creates a new OpAMP Client that uses WebSocket transport.
Expand Down Expand Up @@ -136,7 +141,6 @@ 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 {
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 Down Expand Up @@ -187,6 +191,7 @@ func (c *wsClient) ensureConnected(ctx context.Context) error {
case <-timer.C:
{
if err, retryAfter := c.tryConnectOnce(ctx); err != nil {
c.lastInternalErr.Store(&err)
if errors.Is(err, context.Canceled) {
c.common.Logger.Debugf(ctx, "Client is stopped, will not try anymore.")
return err
Expand Down
6 changes: 4 additions & 2 deletions client/wsclient_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,9 +245,11 @@ func TestRedirectWS(t *testing.T) {
startClient(t, settings, client)

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

0 comments on commit f6cc5df

Please sign in to comment.