Add Sequence field to OpError for better error correlation (#236)

When a message with an errno is returned, it triggers an error in
checkMessage and is not passed back to the caller. This can make it
difficult to match the error to the specific message that caused it
when using conn.SendMessages.

This change includes the message’s Sequence value in OpError, making
it easier to identify which message the error corresponds to.

Author: nickgarlis

conn_linux_error_test.go

@@ -43,8 +43,9 @@ func TestConnReceiveErrorLinux(t *testing.T) {
 				Data: []byte{0xfe, 0xff, 0xff, 0xff},
 			}},
 			want: &netlink.OpError{
-				Op:  "receive",
-				Err: unix.ENOENT,
+				Op:       "receive",
+				Err:      unix.ENOENT,
+				Sequence: 1,
 			},
 		},
 		{

errors.go

@@ -69,6 +69,11 @@ type OpError struct {
 	// is not set, both of these fields will be empty.
 	Message string
 	Offset  int
+
+	// Sequence is the netlink message sequence number associated with the
+	// error. This field is only populated if the error was produced as the
+	// result of a netlink message; otherwise, it will be zero.
+	Sequence uint32
 }
 
 // newOpError is a small wrapper for creating an OpError. As a convenience, it

internal/integration/integration_linux_test.go

@@ -126,13 +126,18 @@ func TestIntegrationRTNetlinkStrictCheckExtendedAcknowledge(t *testing.T) {
 		t.Fatalf("failed to marshal request: %v", err)
 	}
 
-	_, err = c.Execute(netlink.Message{
+	sent, err := c.Send(netlink.Message{
 		Header: netlink.Header{
 			Type:  unix.RTM_GETROUTE,
 			Flags: netlink.Request | netlink.Dump,
 		},
 		Data: b,
 	})
+	if err != nil {
+		t.Fatalf("failed to send request: %v", err)
+	}
+
+	_, err = c.Receive()
 
 	oerr, ok := err.(*netlink.OpError)
 	if !ok {
@@ -143,9 +148,10 @@ func TestIntegrationRTNetlinkStrictCheckExtendedAcknowledge(t *testing.T) {
 	// offset just in case things change.
 
 	want := &netlink.OpError{
-		Op:      "receive",
-		Err:     unix.EINVAL,
-		Message: "Invalid values in header for FIB dump request",
+		Op:       "receive",
+		Err:      unix.EINVAL,
+		Message:  "Invalid values in header for FIB dump request",
+		Sequence: sent.Header.Sequence,
 	}
 
 	if diff := cmp.Diff(want, oerr); diff != "" {
@@ -325,6 +331,9 @@ func TestIntegrationEthtoolExtendedAcknowledge(t *testing.T) {
 	}
 	oerr.Offset = 0
 
+	// Reset Sequence because we do not have access to the sent message.
+	oerr.Sequence = 0
+
 	want := &netlink.OpError{
 		Op:      "receive",
 		Err:     unix.ENODEV,

message.go

@@ -293,7 +293,8 @@ func checkMessage(m Message) error {
 		// Error code is a negative integer, convert it into an OS-specific raw
 		// system call error, but do not wrap with os.NewSyscallError to signify
 		// that this error was produced by a netlink message; not a system call.
-		Err: newError(-1 * int(c)),
+		Err:      newError(-1 * int(c)),
+		Sequence: m.Header.Sequence,
 	}
 
 	// TODO(mdlayher): investigate the Capped flag.

message_linux_test.go

@@ -65,15 +65,19 @@ func Test_checkMessageExtendedAcknowledgementTLVs(t *testing.T) {
 				Header: Header{
 					Type: Error,
 					// Indicate the use of extended acknowledgement.
-					Flags: AcknowledgeTLVs,
+					Flags:    AcknowledgeTLVs,
+					Sequence: 3258842681,
 				},
 				Data: packExtACK(
 					-1,
 					// The caller's request message with arbitrary bytes that we
 					// skip over when parsing the TLVs.
 					&Message{
-						Header: Header{Length: 4},
-						Data:   []byte{0xff, 0xff, 0xff, 0xff},
+						Header: Header{
+							Length:   4,
+							Sequence: 3258842681,
+						},
+						Data: []byte{0xff, 0xff, 0xff, 0xff},
 					},
 					// The actual extended acknowledgement TLVs.
 					[]Attribute{
@@ -89,10 +93,11 @@ func Test_checkMessageExtendedAcknowledgementTLVs(t *testing.T) {
 				),
 			},
 			err: &OpError{
-				Op:      "receive",
-				Err:     unix.Errno(1),
-				Message: "bad request",
-				Offset:  2,
+				Op:       "receive",
+				Err:      unix.Errno(1),
+				Message:  "bad request",
+				Offset:   2,
+				Sequence: 3258842681,
 			},
 		},
 		{
@@ -101,7 +106,8 @@ func Test_checkMessageExtendedAcknowledgementTLVs(t *testing.T) {
 				Header: Header{
 					Type: Done,
 					// Indicate the use of extended acknowledgement.
-					Flags: Multi | AcknowledgeTLVs,
+					Flags:    Multi | AcknowledgeTLVs,
+					Sequence: 3258842681,
 				},
 				Data: packExtACK(
 					-1,
@@ -120,10 +126,11 @@ func Test_checkMessageExtendedAcknowledgementTLVs(t *testing.T) {
 				),
 			},
 			err: &OpError{
-				Op:      "receive",
-				Err:     unix.Errno(1),
-				Message: "bad request",
-				Offset:  2,
+				Op:       "receive",
+				Err:      unix.Errno(1),
+				Message:  "bad request",
+				Offset:   2,
+				Sequence: 3258842681,
 			},
 		},
 	}