From d1c26a20da301ece04315f276725a98093f60436 Mon Sep 17 00:00:00 2001 From: carla Date: Mon, 19 Jul 2021 10:01:33 +0200 Subject: [PATCH 1/2] multi: surface server swap initiation grpc error codes Formatting our error was stifling any grpc error returned by the server. Instead, we bubble up our grpc error, setting an unknown code if the server did not specifically return an error code. --- client.go | 15 +++++++++++++++ client_test.go | 38 ++++++++++++++++++++++++++++++++++++++ loopin.go | 7 +++---- loopin_test.go | 5 ++--- loopout.go | 2 +- 5 files changed, 59 insertions(+), 8 deletions(-) diff --git a/client.go b/client.go index 00edc4a..87c09ca 100644 --- a/client.go +++ b/client.go @@ -15,6 +15,7 @@ import ( "github.com/lightninglabs/loop/loopdb" "github.com/lightninglabs/loop/swap" "github.com/lightninglabs/loop/sweep" + "google.golang.org/grpc/status" ) var ( @@ -610,3 +611,17 @@ func (s *Client) LoopInTerms(ctx context.Context) ( return s.Server.GetLoopInTerms(ctx) } + +// wrapGrpcError wraps the non-nil error provided with a message providing +// additional context, preserving the grpc code returned with the original +// error. If the original error has no grpc code, then codes.Unknown is used. +func wrapGrpcError(message string, err error) error { + // Since our error is non-nil, we don't need to worry about a nil + // grpcStatus, we'll just get an unknown one if no code was passed back. + grpcStatus, _ := status.FromError(err) + + return status.Error( + grpcStatus.Code(), fmt.Sprintf("%v: %v", message, + grpcStatus.Message()), + ) +} diff --git a/client_test.go b/client_test.go index 06995b2..b1d7422 100644 --- a/client_test.go +++ b/client_test.go @@ -17,6 +17,8 @@ import ( "github.com/lightningnetwork/lnd/lnrpc" "github.com/lightningnetwork/lnd/lntypes" "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) var ( @@ -372,3 +374,39 @@ func testSuccess(ctx *testContext, amt btcutil.Amount, hash lntypes.Hash, ctx.finish() } + +// TestWrapGrpcError tests grpc error wrapping in the case where a grpc error +// code is present, and when it is absent. +func TestWrapGrpcError(t *testing.T) { + tests := []struct { + name string + original error + expectedCode codes.Code + }{ + { + name: "out of range error", + original: status.Error( + codes.OutOfRange, "err string", + ), + expectedCode: codes.OutOfRange, + }, + { + name: "no grpc code", + original: errors.New("no error code"), + expectedCode: codes.Unknown, + }, + } + + for _, testCase := range tests { + testCase := testCase + + t.Run(testCase.name, func(t *testing.T) { + err := wrapGrpcError("", testCase.original) + require.Error(t, err, "test only expects errors") + + status, ok := status.FromError(err) + require.True(t, ok, "test expects grpc code") + require.Equal(t, testCase.expectedCode, status.Code()) + }) + } +} diff --git a/loopin.go b/loopin.go index 7a76593..f73fc2c 100644 --- a/loopin.go +++ b/loopin.go @@ -9,11 +9,10 @@ import ( "sync" "time" - "github.com/btcsuite/btcutil" - "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/btcsuite/btcd/mempool" "github.com/btcsuite/btcd/wire" + "github.com/btcsuite/btcutil" "github.com/lightninglabs/lndclient" "github.com/lightninglabs/loop/labels" "github.com/lightninglabs/loop/loopdb" @@ -86,7 +85,7 @@ func newLoopInSwap(globalCtx context.Context, cfg *swapConfig, // request that we send to the server. quote, err := cfg.server.GetLoopInQuote(globalCtx, request.Amount) if err != nil { - return nil, fmt.Errorf("loop in terms: %v", err) + return nil, wrapGrpcError("loop in terms", err) } swapFee := quote.SwapFee @@ -172,7 +171,7 @@ func newLoopInSwap(globalCtx context.Context, cfg *swapConfig, ) probeWaitCancel() if err != nil { - return nil, fmt.Errorf("cannot initiate swap: %v", err) + return nil, wrapGrpcError("cannot initiate swap", err) } // Because the context is cancelled, it is guaranteed that we will be diff --git a/loopin_test.go b/loopin_test.go index 93b6454..888ae7c 100644 --- a/loopin_test.go +++ b/loopin_test.go @@ -4,15 +4,14 @@ import ( "context" "testing" + "github.com/btcsuite/btcd/wire" + "github.com/btcsuite/btcutil" "github.com/lightninglabs/loop/loopdb" "github.com/lightninglabs/loop/swap" "github.com/lightninglabs/loop/test" "github.com/lightningnetwork/lnd/chainntnfs" "github.com/lightningnetwork/lnd/channeldb" "github.com/stretchr/testify/require" - - "github.com/btcsuite/btcd/wire" - "github.com/btcsuite/btcutil" ) var ( diff --git a/loopout.go b/loopout.go index 9c8ce0b..2d4f619 100644 --- a/loopout.go +++ b/loopout.go @@ -128,7 +128,7 @@ func newLoopOutSwap(globalCtx context.Context, cfg *swapConfig, receiverKey, request.SwapPublicationDeadline, request.Initiator, ) if err != nil { - return nil, fmt.Errorf("cannot initiate swap: %v", err) + return nil, wrapGrpcError("cannot initiate swap", err) } err = validateLoopOutContract( From b8f1fd1c4e6e003c42a30b1e6382c0dccfd21058 Mon Sep 17 00:00:00 2001 From: carla Date: Mon, 19 Jul 2021 10:17:24 +0200 Subject: [PATCH 2/2] release_notes: add grpc error code surfacing --- release_notes.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/release_notes.md b/release_notes.md index 9a87269..5123718 100644 --- a/release_notes.md +++ b/release_notes.md @@ -19,3 +19,7 @@ This file tracks release notes for the loop client. #### Breaking Changes #### Bug Fixes +* Grpc error codes returned by the swap server when swap initiation fails are + now surfaced to the client. Previously these error codes would be returned + as a string. +