From 33fe09482b70db2f09f5680ee1ef46641e139ecf Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Fri, 20 Sep 2019 10:55:21 +0200 Subject: [PATCH] lnwire+multi: define Error() for lnwire.Error To make lnwire.Error actually satisfy the error interface, define the Error method directly. --- fundingmanager.go | 4 ++-- fundingmanager_test.go | 26 +++++++++++++------------- htlcswitch/link.go | 19 +------------------ lnwire/error.go | 24 ++++++++++++++++++++++++ peer.go | 2 +- 5 files changed, 41 insertions(+), 34 deletions(-) diff --git a/fundingmanager.go b/fundingmanager.go index 34b0a8f1..04c41f09 100644 --- a/fundingmanager.go +++ b/fundingmanager.go @@ -2955,14 +2955,14 @@ func (f *fundingManager) handleErrorMsg(fmsg *fundingErrorMsg) { resCtx, err := f.cancelReservationCtx(fmsg.peerKey, chanID) if err != nil { fndgLog.Warnf("Received error for non-existent funding "+ - "flow: %v (%v)", err, spew.Sdump(protocolErr)) + "flow: %v (%v)", err, protocolErr.Error()) return } // If we did indeed find the funding workflow, then we'll return the // error back to the caller (if any), and cancel the workflow itself. fundingErr := fmt.Errorf("received funding error from %x: %v", - fmsg.peerKey.SerializeCompressed(), string(protocolErr.Data), + fmsg.peerKey.SerializeCompressed(), protocolErr.Error(), ) fndgLog.Errorf(fundingErr.Error()) diff --git a/fundingmanager_test.go b/fundingmanager_test.go index e53ed045..4fb31ee8 100644 --- a/fundingmanager_test.go +++ b/fundingmanager_test.go @@ -602,7 +602,7 @@ func fundChannel(t *testing.T, alice, bob *testNode, localFundingAmt, if gotError { t.Fatalf("expected OpenChannel to be sent "+ "from bob, instead got error: %v", - lnwire.ErrorCode(errorMsg.Data[0])) + errorMsg.Error()) } t.Fatalf("expected OpenChannel to be sent from "+ "alice, instead got %T", aliceMsg) @@ -728,7 +728,7 @@ func assertFundingMsgSent(t *testing.T, msgChan chan lnwire.Message, errorMsg, gotError := msg.(*lnwire.Error) if gotError { t.Fatalf("expected %s to be sent, instead got error: %v", - msgType, lnwire.ErrorCode(errorMsg.Data[0])) + msgType, errorMsg.Error()) } _, _, line, _ := runtime.Caller(1) @@ -1469,7 +1469,7 @@ func TestFundingManagerPeerTimeoutAfterInitFunding(t *testing.T) { if gotError { t.Fatalf("expected OpenChannel to be sent "+ "from bob, instead got error: %v", - lnwire.ErrorCode(errorMsg.Data[0])) + errorMsg.Error()) } t.Fatalf("expected OpenChannel to be sent from "+ "alice, instead got %T", aliceMsg) @@ -1531,7 +1531,7 @@ func TestFundingManagerPeerTimeoutAfterFundingOpen(t *testing.T) { if gotError { t.Fatalf("expected OpenChannel to be sent "+ "from bob, instead got error: %v", - lnwire.ErrorCode(errorMsg.Data[0])) + errorMsg.Error()) } t.Fatalf("expected OpenChannel to be sent from "+ "alice, instead got %T", aliceMsg) @@ -1602,7 +1602,7 @@ func TestFundingManagerPeerTimeoutAfterFundingAccept(t *testing.T) { if gotError { t.Fatalf("expected OpenChannel to be sent "+ "from bob, instead got error: %v", - lnwire.ErrorCode(errorMsg.Data[0])) + errorMsg.Error()) } t.Fatalf("expected OpenChannel to be sent from "+ "alice, instead got %T", aliceMsg) @@ -2326,7 +2326,7 @@ func TestFundingManagerCustomChannelParameters(t *testing.T) { if gotError { t.Fatalf("expected OpenChannel to be sent "+ "from bob, instead got error: %v", - lnwire.ErrorCode(errorMsg.Data[0])) + errorMsg.Error()) } t.Fatalf("expected OpenChannel to be sent from "+ "alice, instead got %T", aliceMsg) @@ -2561,7 +2561,7 @@ func TestFundingManagerMaxPendingChannels(t *testing.T) { if gotError { t.Fatalf("expected OpenChannel to be sent "+ "from bob, instead got error: %v", - lnwire.ErrorCode(errorMsg.Data[0])) + errorMsg.Error()) } t.Fatalf("expected OpenChannel to be sent from "+ "alice, instead got %T", aliceMsg) @@ -2725,7 +2725,7 @@ func TestFundingManagerRejectPush(t *testing.T) { if gotError { t.Fatalf("expected OpenChannel to be sent "+ "from bob, instead got error: %v", - lnwire.ErrorCode(errorMsg.Data[0])) + errorMsg.Error()) } t.Fatalf("expected OpenChannel to be sent from "+ "alice, instead got %T", aliceMsg) @@ -2736,9 +2736,9 @@ func TestFundingManagerRejectPush(t *testing.T) { // Assert Bob responded with an ErrNonZeroPushAmount error. err := assertFundingMsgSent(t, bob.msgChan, "Error").(*lnwire.Error) - if "Non-zero push amounts are disabled" != string(err.Data) { + if !strings.Contains(err.Error(), "Non-zero push amounts are disabled") { t.Fatalf("expected ErrNonZeroPushAmount error, got \"%v\"", - string(err.Data)) + err.Error()) } } @@ -2782,7 +2782,7 @@ func TestFundingManagerMaxConfs(t *testing.T) { if gotError { t.Fatalf("expected OpenChannel to be sent "+ "from bob, instead got error: %v", - lnwire.ErrorCode(errorMsg.Data[0])) + errorMsg.Error()) } t.Fatalf("expected OpenChannel to be sent from "+ "alice, instead got %T", aliceMsg) @@ -2805,9 +2805,9 @@ func TestFundingManagerMaxConfs(t *testing.T) { // Alice should respond back with an error indicating MinAcceptDepth is // too large. err := assertFundingMsgSent(t, alice.msgChan, "Error").(*lnwire.Error) - if !strings.Contains(string(err.Data), "minimum depth") { + if !strings.Contains(err.Error(), "minimum depth") { t.Fatalf("expected ErrNumConfsTooLarge, got \"%v\"", - string(err.Data)) + err.Error()) } } diff --git a/htlcswitch/link.go b/htlcswitch/link.go index 64e6dd00..c4603d4b 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -1882,13 +1882,9 @@ func (l *channelLink) handleUpstreamMsg(msg lnwire.Message) { // Error received from remote, MUST fail channel, but should // only print the contents of the error message if all // characters are printable ASCII. - errMsg := "non-ascii data" - if isASCII(msg.Data) { - errMsg = string(msg.Data) - } l.fail(LinkFailureError{code: ErrRemoteError}, "ChannelPoint(%v): received error from peer: %v", - l.channel.ChannelPoint(), errMsg) + l.channel.ChannelPoint(), msg.Error()) default: log.Warnf("ChannelPoint(%v): received unknown message of type %T", l.channel.ChannelPoint(), msg) @@ -3100,16 +3096,3 @@ func (l *channelLink) tracef(format string, a ...interface{}) { msg := fmt.Sprintf(format, a...) log.Tracef("ChannelLink(%s) %s", l.ShortChanID(), msg) } - -// isASCII is a helper method that checks whether all bytes in `data` would be -// printable ASCII characters if interpreted as a string. -func isASCII(data []byte) bool { - isASCII := true - for _, c := range data { - if c < 32 || c > 126 { - isASCII = false - break - } - } - return isASCII -} diff --git a/lnwire/error.go b/lnwire/error.go index 5a4dd1bf..c4159e7a 100644 --- a/lnwire/error.go +++ b/lnwire/error.go @@ -1,6 +1,7 @@ package lnwire import ( + "fmt" "io" "google.golang.org/grpc/codes" @@ -87,6 +88,18 @@ func NewError() *Error { // interface. var _ Message = (*Error)(nil) +// Error returns the string representation to Error. +// +// NOTE: Satisfies the error interface. +func (c *Error) Error() string { + errMsg := "non-ascii data" + if isASCII(c.Data) { + errMsg = string(c.Data) + } + + return fmt.Sprintf("chan_id=%v, err=%v", c.ChanID, errMsg) +} + // Decode deserializes a serialized Error message stored in the passed // io.Reader observing the specified protocol version. // @@ -125,3 +138,14 @@ func (c *Error) MaxPayloadLength(uint32) uint32 { // 32 + 2 + 65501 return 65535 } + +// isASCII is a helper method that checks whether all bytes in `data` would be +// printable ASCII characters if interpreted as a string. +func isASCII(data []byte) bool { + for _, c := range data { + if c < 32 || c > 126 { + return false + } + } + return true +} diff --git a/peer.go b/peer.go index 2b4460a4..247daab7 100644 --- a/peer.go +++ b/peer.go @@ -1250,7 +1250,7 @@ func messageSummary(msg lnwire.Message) string { msg.ChanID, msg.ID, msg.FailureCode) case *lnwire.Error: - return fmt.Sprintf("chan_id=%v, err=%v", msg.ChanID, string(msg.Data)) + return fmt.Sprintf("%v", msg.Error()) case *lnwire.AnnounceSignatures: return fmt.Sprintf("chan_id=%v, short_chan_id=%v", msg.ChannelID,