light: fix provider error plumbing (#7610)

The custom error types in the provider package did not propagate their wrapped
underlying reasons, making it difficult for the test to check that the correct
error was observed.

- Fix the custom errors to have a true underlying error (not just a string).
- Add Unwrap methods to support inspection by errors.Is.
- Update usage in a few places.
- Fix the test to check for acceptable variation.

Fixes #7609.

internal/statesync/dispatcher.go

@@ -195,7 +195,7 @@ func (p *BlockProvider) LightBlock(ctx context.Context, height int64) (*types.Li
 	case errPeerAlreadyBusy:
 		return nil, provider.ErrLightBlockNotFound
 	default:
-		return nil, provider.ErrUnreliableProvider{Reason: err.Error()}
+		return nil, provider.ErrUnreliableProvider{Reason: err}
 	}
 
 	// check that the height requested is the same one returned

light/provider/errors.go

@@ -28,16 +28,20 @@ type ErrBadLightBlock struct {
 }
 
 func (e ErrBadLightBlock) Error() string {
-	return fmt.Sprintf("client provided bad signed header: %s", e.Reason.Error())
+	return fmt.Sprintf("client provided bad signed header: %v", e.Reason)
 }
 
+func (e ErrBadLightBlock) Unwrap() error { return e.Reason }
+
 // ErrUnreliableProvider is a generic error that indicates that the provider isn't
 // behaving in a reliable manner to the light client. The light client will
 // remove the provider
 type ErrUnreliableProvider struct {
-	Reason string
+	Reason error
 }
 
 func (e ErrUnreliableProvider) Error() string {
-	return fmt.Sprintf("client deemed unreliable: %s", e.Reason)
+	return fmt.Sprintf("client deemed unreliable: %v", e.Reason)
 }
+
+func (e ErrUnreliableProvider) Unwrap() error { return e.Reason }

light/provider/http/http.go

@@ -212,7 +212,7 @@ func (p *http) validatorSet(ctx context.Context, height *int64) (*types.Validato
 
 				// If we don't know the error then by default we return an unreliable provider error and
 				// terminate the connection with the peer.
-				return nil, provider.ErrUnreliableProvider{Reason: e.Error()}
+				return nil, provider.ErrUnreliableProvider{Reason: e}
 			}
 
 			// update the total and increment the page index so we can fetch the
@@ -268,7 +268,7 @@ func (p *http) signedHeader(ctx context.Context, height *int64) (*types.SignedHe
 
 			// If we don't know the error then by default we return an unreliable provider error and
 			// terminate the connection with the peer.
-			return nil, provider.ErrUnreliableProvider{Reason: e.Error()}
+			return nil, provider.ErrUnreliableProvider{Reason: e}
 		}
 	}
 	return nil, p.noResponse()
@@ -278,7 +278,7 @@ func (p *http) noResponse() error {
 	p.noResponseCount++
 	if p.noResponseCount > p.noResponseThreshold {
 		return provider.ErrUnreliableProvider{
-			Reason: fmt.Sprintf("failed to respond after %d attempts", p.noResponseCount),
+			Reason: fmt.Errorf("failed to respond after %d attempts", p.noResponseCount),
 		}
 	}
 	return provider.ErrNoResponse
@@ -288,7 +288,7 @@ func (p *http) noBlock(e error) error {
 	p.noBlockCount++
 	if p.noBlockCount > p.noBlockThreshold {
 		return provider.ErrUnreliableProvider{
-			Reason: fmt.Sprintf("failed to provide a block after %d attempts", p.noBlockCount),
+			Reason: fmt.Errorf("failed to provide a block after %d attempts", p.noBlockCount),
 		}
 	}
 	return e

light/provider/http/http_test.go

@@ -2,6 +2,7 @@ package http_test
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"testing"
 	"time"
@@ -100,8 +101,10 @@ func TestProvider(t *testing.T) {
 
 	time.Sleep(10 * time.Second)
 	lb, err = p.LightBlock(ctx, lower+2)
-	// we should see a connection refused
+	// Either the connection should be refused, or the context canceled.
 	require.Error(t, err)
 	require.Nil(t, lb)
-	assert.Equal(t, provider.ErrConnectionClosed, err)
+	if !errors.Is(err, provider.ErrConnectionClosed) && !errors.Is(err, context.Canceled) {
+		assert.Fail(t, "Incorrect error", "wanted connection closed or context canceled, got %v", err)
+	}
 }