Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there an advantage of wrapping unknown errors in the few places in the PR vs just returning status.Error there with proper error codes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's nice to be able to wrap with a bit more information and preserve the error? In the case of unknown errors, I think it's ok to add the unknown error code anyway, doesn't really take away much. We could go through the whole api and add error codes everywhere, but I think surfacing the error codes returned by the server is a good start?

// 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()),
)
}
38 changes: 38 additions & 0 deletions client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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())
})
}
}
7 changes: 3 additions & 4 deletions loopin.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
5 changes: 2 additions & 3 deletions loopin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
2 changes: 1 addition & 1 deletion loopout.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
4 changes: 4 additions & 0 deletions release_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.