chore: do not use sentinel errors when unneeded

- main reason being it's a burden to always define sentinel errors at global scope, wrap them with `%w` instead of using a string directly
- only use sentinel errors when it has to be checked using `errors.Is`
- replace all usage of these sentinel errors in `fmt.Errorf` with direct strings that were in the sentinel error
- exclude the sentinel error definition requirement from .golangci.yml
- update unit tests to use ContainersError instead of ErrorIs so it stays as a "not a change detector test" without requiring a sentinel error
This commit is contained in:
Quentin McGaw
2026-05-02 00:50:16 +00:00
parent 9b6f048fe8
commit 4a78989d9d
172 changed files with 666 additions and 1433 deletions
+10 -21
View File
@@ -2,24 +2,17 @@ package icmp
import (
"bytes"
"errors"
"fmt"
"golang.org/x/net/icmp"
)
var (
ErrNextHopMTUTooLow = errors.New("ICMP Next Hop MTU is too low")
ErrNextHopMTUTooHigh = errors.New("ICMP Next Hop MTU is too high")
)
func checkMTU(mtu, minMTU, physicalLinkMTU uint32) (err error) {
switch {
case mtu < minMTU:
return fmt.Errorf("%w: %d", ErrNextHopMTUTooLow, mtu)
return fmt.Errorf("ICMP Next Hop MTU is too low: %d", mtu)
case mtu > physicalLinkMTU:
return fmt.Errorf("%w: %d is larger than physical link MTU %d",
ErrNextHopMTUTooHigh, mtu, physicalLinkMTU)
return fmt.Errorf("ICMP Next Hop MTU is too high: %d is larger than physical link MTU %d", mtu, physicalLinkMTU)
default:
return nil
}
@@ -34,14 +27,12 @@ func checkInvokingReplyIDMatch(icmpProtocol int, received []byte,
}
inboundBody, ok := inboundMessage.Body.(*icmp.Echo)
if !ok {
return false, fmt.Errorf("%w: %T", ErrBodyUnsupported, inboundMessage.Body)
return false, fmt.Errorf("ICMP body type is not supported: %T", inboundMessage.Body)
}
outboundBody := outboundMessage.Body.(*icmp.Echo) //nolint:forcetypeassert
return inboundBody.ID == outboundBody.ID, nil
}
var ErrIDMismatch = errors.New("ICMP id mismatch")
func checkEchoReply(icmpProtocol int, received []byte,
outboundMessage *icmp.Message, truncatedBody bool,
) (err error) {
@@ -51,12 +42,12 @@ func checkEchoReply(icmpProtocol int, received []byte,
}
inboundBody, ok := inboundMessage.Body.(*icmp.Echo)
if !ok {
return fmt.Errorf("%w: %T", ErrBodyUnsupported, inboundMessage.Body)
return fmt.Errorf("ICMP body type is not supported: %T", inboundMessage.Body)
}
outboundBody := outboundMessage.Body.(*icmp.Echo) //nolint:forcetypeassert
if inboundBody.ID != outboundBody.ID {
return fmt.Errorf("%w: sent id %d and received id %d",
ErrIDMismatch, outboundBody.ID, inboundBody.ID)
return fmt.Errorf("ICMP id mismatch: sent id %d and received id %d",
outboundBody.ID, inboundBody.ID)
}
err = checkEchoBodies(outboundBody.Data, inboundBody.Data, truncatedBody)
if err != nil {
@@ -65,19 +56,17 @@ func checkEchoReply(icmpProtocol int, received []byte,
return nil
}
var ErrEchoDataMismatch = errors.New("ICMP data mismatch")
func checkEchoBodies(sent, received []byte, receivedTruncated bool) (err error) {
if len(received) > len(sent) {
return fmt.Errorf("%w: sent %d bytes and received %d bytes",
ErrEchoDataMismatch, len(sent), len(received))
return fmt.Errorf("ICMP data mismatch: sent %d bytes and received %d bytes",
len(sent), len(received))
}
if receivedTruncated {
sent = sent[:len(received)]
}
if !bytes.Equal(received, sent) {
return fmt.Errorf("%w: sent %x and received %x",
ErrEchoDataMismatch, sent, received)
return fmt.Errorf("ICMP data mismatch: sent %x and received %x",
sent, received)
}
return nil
}
+1 -3
View File
@@ -10,9 +10,7 @@ import (
var (
ErrNotPermitted = errors.New("ICMP not permitted")
ErrDestinationUnreachable = errors.New("ICMP destination unreachable")
ErrCommunicationAdministrativelyProhibited = errors.New("communication administratively prohibited")
ErrBodyUnsupported = errors.New("ICMP body type is not supported")
errCommunicationAdministrativelyProhibited = errors.New("communication administratively prohibited")
ErrMTUNotFound = errors.New("MTU not found")
errTimeout = errors.New("operation timed out")
)
+1 -1
View File
@@ -25,7 +25,7 @@ func PathMTUDiscover(ctx context.Context, ip netip.Addr,
switch {
case err == nil:
return mtu, nil
case errors.Is(err, errTimeout) || errors.Is(err, ErrCommunicationAdministrativelyProhibited): // blackhole
case errors.Is(err, errTimeout) || errors.Is(err, errCommunicationAdministrativelyProhibited): // blackhole
default:
return 0, fmt.Errorf("finding IPv4 next hop MTU to %s: %w", ip, err)
}
+3 -6
View File
@@ -117,13 +117,10 @@ func findIPv4NextHopMTU(ctx context.Context, ip netip.Addr,
case portUnreachable: // triggered by TCP or UDP from applications
continue // ignore and wait for the next message
case communicationAdministrativelyProhibitedCode:
return 0, fmt.Errorf("%w: %w (code %d)",
ErrDestinationUnreachable,
ErrCommunicationAdministrativelyProhibited,
return 0, fmt.Errorf("ICMP destination unreachable: %w (code %d)", errCommunicationAdministrativelyProhibited,
inboundMessage.Code)
default:
return 0, fmt.Errorf("%w: code %d",
ErrDestinationUnreachable, inboundMessage.Code)
return 0, fmt.Errorf("ICMP destination unreachable: code %d", inboundMessage.Code)
}
// See https://datatracker.ietf.org/doc/html/rfc1191#section-4
@@ -158,7 +155,7 @@ func findIPv4NextHopMTU(ctx context.Context, ip netip.Addr,
inboundID, outboundID)
continue
default:
return 0, fmt.Errorf("%w: %T", ErrBodyUnsupported, typedBody)
return 0, fmt.Errorf("ICMP body type is not supported: %T", typedBody)
}
}
}
+3 -2
View File
@@ -2,6 +2,7 @@ package icmp
import (
"context"
"errors"
"fmt"
"net"
"net/netip"
@@ -115,7 +116,7 @@ func getIPv6PacketTooBig(ctx context.Context, ip netip.Addr,
if err != nil {
return 0, fmt.Errorf("checking invoking message id: %w", err)
} else if idMatch {
return 0, fmt.Errorf("%w", ErrDestinationUnreachable)
return 0, errors.New("ICMP destination unreachable")
}
logger.Debug("discarding received ICMP destination unreachable reply with an unknown id")
continue
@@ -128,7 +129,7 @@ func getIPv6PacketTooBig(ctx context.Context, ip netip.Addr,
inboundID, outboundID)
continue
default:
return 0, fmt.Errorf("%w: %T", ErrBodyUnsupported, typedBody)
return 0, fmt.Errorf("ICMP body type %T is not supported", typedBody)
}
}
}
+4 -4
View File
@@ -71,7 +71,7 @@ func pmtudMultiSizes(ctx context.Context, ip netip.Addr,
_, err = conn.WriteTo(encodedMessage, &net.IPAddr{IP: ip.AsSlice()})
if err != nil {
if strings.HasSuffix(err.Error(), "sendto: operation not permitted") {
err = fmt.Errorf("%w", ErrNotPermitted)
err = ErrNotPermitted
}
return 0, fmt.Errorf("writing ICMP message: %w", err)
}
@@ -157,7 +157,7 @@ func collectReplies(conn net.PacketConn, ipVersion string,
logger.Debugf("ignoring ICMP message (type: %d, code: %d)", message.Type, message.Code)
continue
default:
return fmt.Errorf("%w: %T", ErrBodyUnsupported, message.Body)
return fmt.Errorf("ICMP body type is not supported: %T", message.Body)
}
echoBody, _ := message.Body.(*icmp.Echo)
@@ -183,8 +183,8 @@ func collectReplies(conn net.PacketConn, ipVersion string,
ipPacketLength == conservativeReplyLength
// Check the packet size is the same if the reply is not truncated
if !truncated && sentBytes != ipPacketLength {
return fmt.Errorf("%w: sent %dB and received %dB",
ErrEchoDataMismatch, sentBytes, ipPacketLength)
return fmt.Errorf("ICMP data mismatch: sent %dB and received %dB",
sentBytes, ipPacketLength)
}
// Truncated reply or matching reply size
tests[testIndex].ok = true
+1 -1
View File
@@ -29,7 +29,7 @@ func SrcAddr(dst netip.AddrPort, proto int) (src netip.AddrPort, cleanup func(),
}
var (
errNoRoute = fmt.Errorf("no route to destination")
errNoRoute = errors.New("no route to destination")
ErrNetworkUnreachable = errors.New("network unreachable")
)
+3 -8
View File
@@ -13,11 +13,6 @@ import (
"github.com/qdm12/gluetun/internal/pmtud/tcp"
)
var (
ErrICMPOkTCPFail = errors.New("PMTUD succeeded with ICMP but failed with TCP")
ErrICMPFailTCPFail = errors.New("PMTUD failed with both ICMP and TCP")
)
// PathMTUDiscover discovers the maximum MTU using both ICMP and TCP.
// Multiple ICMP addresses and TCP addresses can be specified for redundancy.
// ICMP PMTUD is run first. If successful, the range of possible MTU values to
@@ -81,10 +76,10 @@ func PathMTUDiscover(ctx context.Context, icmpAddrs []netip.Addr, tcpAddrs []net
}
}
if icmpSuccess {
return 0, fmt.Errorf("%w - discarding ICMP obtained MTU %d",
ErrICMPOkTCPFail, maxPossibleMTU)
return 0, fmt.Errorf("PMTUD succeeded with ICMP but failed with TCP "+
"- discarding ICMP obtained MTU %d", maxPossibleMTU)
}
return 0, fmt.Errorf("%w", ErrICMPFailTCPFail)
return 0, errors.New("PMTUD failed with both ICMP and TCP")
}
logger.Debugf("TCP path MTU discovery found maximum valid MTU %d", mtu)
return mtu, nil
+2 -4
View File
@@ -57,8 +57,6 @@ func (l *noopLogger) Warn(_ string) {}
func (l *noopLogger) Warnf(_ string, _ ...any) {}
func (l *noopLogger) Error(_ string) {}
var errRouteNotFound = errors.New("route not found")
func findLoopbackMTU(netlinker *netlink.NetLink) (mtu uint32, err error) {
routes, err := netlinker.RouteList(netlink.FamilyV4)
if err != nil {
@@ -76,7 +74,7 @@ func findLoopbackMTU(netlinker *netlink.NetLink) (mtu uint32, err error) {
return min(link.MTU, maxMTU), nil
}
}
return 0, fmt.Errorf("%w: no loopback route found", errRouteNotFound)
return 0, errors.New("route not found: no loopback route found")
}
func findDefaultRouteMTU(netlinker *netlink.NetLink) (mtu uint32, err error) {
@@ -100,7 +98,7 @@ func findDefaultRouteMTU(netlinker *netlink.NetLink) (mtu uint32, err error) {
}
}
if mtu == 0 {
return 0, fmt.Errorf("%w: no default route found", errRouteNotFound)
return 0, errors.New("route not found: no default route found")
}
return mtu, nil
}
+5 -8
View File
@@ -12,8 +12,6 @@ import (
"github.com/qdm12/gluetun/internal/pmtud/ip"
)
var errTCPServersUnreachable = errors.New("all TCP servers are unreachable")
// findHighestMSSDestination finds the destination with the highest
// MSS amongst the provided destinations.
func findHighestMSSDestination(ctx context.Context, familyToFD map[int]fileDescriptor,
@@ -68,7 +66,7 @@ func findHighestMSSDestination(ctx context.Context, familyToFD map[int]fileDescr
}
if mss == 0 { // no MSS found for any destination
return netip.AddrPort{}, 0, fmt.Errorf("%w (%d servers)", errTCPServersUnreachable, len(dsts))
return netip.AddrPort{}, 0, fmt.Errorf("all %d TCP servers are unreachable", len(dsts))
}
maxPossibleMTU = ip.HeaderLength(dst.Addr().Is4()) + constants.BaseTCPHeaderLength + mss
@@ -77,8 +75,6 @@ func findHighestMSSDestination(ctx context.Context, familyToFD map[int]fileDescr
return dst, mss, nil
}
var errMSSNotFound = errors.New("MSS option not found in reply")
func findMSS(ctx context.Context, fd fileDescriptor, dst netip.AddrPort,
excludeMark int, tracker *tracker, firewall Firewall, logger Logger) (
mss uint32, err error,
@@ -132,11 +128,12 @@ func findMSS(ctx context.Context, fd fileDescriptor, dst netip.AddrPort,
case err != nil:
return 0, fmt.Errorf("parsing reply TCP header: %w", err)
case replyHeader.typ != packetTypeSYNACK:
return 0, fmt.Errorf("%w: unexpected packet type %s", errTCPPacketNotSynAck, replyHeader.typ)
return 0, fmt.Errorf("TCP packet is not a SYN-ACK: unexpected packet type %s", replyHeader.typ)
case replyHeader.ack != synSeq+1:
return 0, fmt.Errorf("%w: expected %d, got %d", errTCPSynAckAckMismatch, synSeq+1, replyHeader.ack)
return 0, fmt.Errorf("TCP SYN-ACK ACK number %d does not match expected value %d",
replyHeader.ack, synSeq+1)
case replyHeader.options.mss == 0:
return 0, fmt.Errorf("%w: MSS option not found in reply", errMSSNotFound)
return 0, errors.New("MSS option not found in reply")
}
err = sendRST(fd, src, dst, replyHeader.ack)
+1 -6
View File
@@ -12,11 +12,6 @@ import (
"github.com/qdm12/gluetun/internal/pmtud/test"
)
var (
ErrMTUNotFound = errors.New("MTU not found")
ErrMSSTooSmall = errors.New("TCP MSS is too small to find the MTU")
)
type testUnit struct {
mtu uint32
ok bool
@@ -178,5 +173,5 @@ func pathMTUDiscover(ctx context.Context, fd fileDescriptor,
}
}
return 0, fmt.Errorf("%w: your connection might not be working at all", ErrMTUNotFound)
return 0, errors.New("MTU not found: your connection might not be working at all")
}
+6 -14
View File
@@ -75,13 +75,6 @@ func startRawSocket(family, excludeMark int) (fd fileDescriptor, stop func(), er
return fileDescriptor(fdPlatform), stop, nil
}
var (
errTCPPacketNotSynAck = errors.New("TCP packet is not a SYN-ACK")
errTCPSynAckAckMismatch = errors.New("TCP SYN-ACK ACK number does not match expected value")
errFinalPacketTypeUnexpected = errors.New("final TCP packet type is unexpected")
errTCPPacketLost = errors.New("TCP packet was lost")
)
// Craft and send a raw TCP packet to test the MTU.
// It expects either an RST reply (if no server is listening)
// or a SYN-ACK/ACK reply (if a server is listening).
@@ -142,9 +135,10 @@ func runTest(ctx context.Context, dst netip.AddrPort, mtu uint32,
// server actively closed the connection, try sending a SYN with data
return handleRSTReply(ctx, fd, ch, src, dst, mtu)
case firstReplyHeader.typ != packetTypeSYNACK:
return fmt.Errorf("%w: unexpected packet type %s", errTCPPacketNotSynAck, firstReplyHeader.typ)
return fmt.Errorf("TCP packet is not a SYN-ACK: unexpected packet type %s", firstReplyHeader.typ)
case firstReplyHeader.ack != synSeq+1:
return fmt.Errorf("%w: expected %d, got %d", errTCPSynAckAckMismatch, synSeq+1, firstReplyHeader.ack)
return fmt.Errorf("TCP SYN-ACK ACK number does not match expected value: "+
"expected %d, got %d", synSeq+1, firstReplyHeader.ack)
}
if firstReplyHeader.options.mss != 0 {
@@ -191,15 +185,13 @@ func runTest(ctx context.Context, dst netip.AddrPort, mtu uint32,
}
return nil
case packetTypeSYNACK: // server never received our MTU-test ACK packet
return fmt.Errorf("%w: server responded with second SYN-ACK packet", errTCPPacketLost)
return errors.New("TCP packet was lost: server responded with second SYN-ACK packet")
default:
_ = sendRST(fd, src, dst, finalPacketHeader.ack)
return fmt.Errorf("%w: %s", errFinalPacketTypeUnexpected, finalPacketHeader.typ)
return fmt.Errorf("final TCP packet type is unexpected: %s", finalPacketHeader.typ)
}
}
var errTCPPacketNotRST = errors.New("TCP packet is not an RST")
func handleRSTReply(ctx context.Context, fd fileDescriptor, ch <-chan []byte,
src, dst netip.AddrPort, mtu uint32,
) error {
@@ -223,7 +215,7 @@ func handleRSTReply(ctx context.Context, fd fileDescriptor, ch <-chan []byte,
return fmt.Errorf("parsing reply TCP header: %w", err)
} else if replyPacketHeader.typ != packetTypeRST &&
replyPacketHeader.typ != packetTypeRSTACK {
return fmt.Errorf("%w: %s", errTCPPacketNotRST, replyPacketHeader.typ)
return fmt.Errorf("TCP packet is not an RST: %s", replyPacketHeader.typ)
}
return nil
}
+18 -34
View File
@@ -2,7 +2,6 @@ package tcp
import (
"encoding/binary"
"errors"
"fmt"
"github.com/qdm12/gluetun/internal/pmtud/constants"
@@ -120,17 +119,11 @@ type tcpHeader struct {
options options
}
var (
errTCPHeaderTooShort = errors.New("TCP header is too short")
errTCPHeaderDataOffset = errors.New("TCP header data offset is invalid")
errTCPPacketTypeUnknown = errors.New("TCP packet type is unknown")
)
// parseTCPHeader parses the TCP header from b.
// b should be the entire TCP packet bytes.
func parseTCPHeader(b []byte) (header tcpHeader, err error) {
if len(b) < int(constants.BaseTCPHeaderLength) {
return tcpHeader{}, fmt.Errorf("%w: %d bytes", errTCPHeaderTooShort, len(b))
return tcpHeader{}, fmt.Errorf("TCP header is too short: %d bytes", len(b))
}
header.srcPort = binary.BigEndian.Uint16(b[0:2])
@@ -146,11 +139,11 @@ func parseTCPHeader(b []byte) (header tcpHeader, err error) {
switch {
case uint32(header.dataOffset) < constants.BaseTCPHeaderLength:
return tcpHeader{}, fmt.Errorf("%w: data offset is %d bytes, expected at least %d bytes",
errTCPHeaderDataOffset, header.dataOffset, constants.BaseTCPHeaderLength)
return tcpHeader{}, fmt.Errorf("TCP header data offset is invalid: "+
"data offset is %d bytes, expected at least %d bytes", header.dataOffset, constants.BaseTCPHeaderLength)
case int(header.dataOffset) > len(b):
return tcpHeader{}, fmt.Errorf("%w: data offset is %d bytes, but packet is only %d bytes",
errTCPHeaderDataOffset, header.dataOffset, len(b))
return tcpHeader{}, fmt.Errorf("TCP header data offset is invalid: "+
"data offset is %d bytes, but packet is only %d bytes", header.dataOffset, len(b))
}
if uint32(header.dataOffset) > constants.BaseTCPHeaderLength {
@@ -186,7 +179,7 @@ func parseTCPHeader(b []byte) (header tcpHeader, err error) {
case flags&ackFlag != 0:
header.typ = packetTypeACK
default:
return tcpHeader{}, fmt.Errorf("%w: flags are 0x%02x", errTCPPacketTypeUnknown, flags)
return tcpHeader{}, fmt.Errorf("TCP packet type is unknown: flags are 0x%02x", flags)
}
header.seq = binary.BigEndian.Uint32(b[4:8])
@@ -206,15 +199,6 @@ type optionTimestamps struct {
echo uint32
}
var (
errTCPOptionLengthTruncated = errors.New("TCP option length is truncated")
ErrTCPOptionLengthInvalid = errors.New("TCP option length is invalid")
ErrTCPOptionMSSInvalid = errors.New("TCP option MSS value is invalid")
ErrTCPOptionWindowScaleInvalid = errors.New("TCP option Window Scale value is invalid")
ErrTCPOptionTimestampsInvalid = errors.New("TCP option Timestamps value is invalid")
errTCPOptionTypeUnknown = errors.New("TCP option type is unknown")
)
func parseTCPOptions(b []byte) (parsed options, err error) {
i := 0
for i < len(b) {
@@ -232,7 +216,7 @@ func parseTCPOptions(b []byte) (parsed options, err error) {
// Handle TLV (Type-Length-Value) options
if i+1 >= len(b) {
// This should not happen for DF packets.
return options{}, fmt.Errorf("%w: at offset %d", errTCPOptionLengthTruncated, i)
return options{}, fmt.Errorf("TCP option length is truncated: at offset %d", i)
}
length := int(b[i+1])
@@ -240,11 +224,11 @@ func parseTCPOptions(b []byte) (parsed options, err error) {
maxLength := len(b) - i
switch {
case length < minLength:
return options{}, fmt.Errorf("%w: type %d at offset %d has length %d < %d",
ErrTCPOptionLengthInvalid, optionType, i, length, minLength)
return options{}, fmt.Errorf("TCP option length is invalid: "+
"type %d at offset %d has length %d < %d", optionType, i, length, minLength)
case length > maxLength:
return options{}, fmt.Errorf("%w: type %d at offset %d has length %d > %d",
ErrTCPOptionLengthInvalid, optionType, i, length, maxLength)
return options{}, fmt.Errorf("TCP option length is invalid: "+
"type %d at offset %d has length %d > %d", optionType, i, length, maxLength)
}
data := b[i+2 : i+length]
@@ -259,15 +243,15 @@ func parseTCPOptions(b []byte) (parsed options, err error) {
case optionTypeMSS:
const expectedLength = 4
if length != expectedLength {
return options{}, fmt.Errorf("%w: MSS option at offset %d has length %d, expected %d",
ErrTCPOptionMSSInvalid, i, length, expectedLength)
return options{}, fmt.Errorf("TCP option MSS value is invalid: "+
"MSS option at offset %d has length %d, expected %d", i, length, expectedLength)
}
parsed.mss = uint32(binary.BigEndian.Uint16(data))
case optionTypeWindowScale:
const expectedLength = 3
if length != expectedLength {
return options{}, fmt.Errorf("%w: window scale option at offset %d has length %d, expected %d",
ErrTCPOptionWindowScaleInvalid, i, length, expectedLength)
return options{}, fmt.Errorf("TCP option Window Scale value is invalid: "+
"window scale option at offset %d has length %d, expected %d", i, length, expectedLength)
}
windowScale := data[0]
parsed.windowScale = &windowScale
@@ -276,15 +260,15 @@ func parseTCPOptions(b []byte) (parsed options, err error) {
case optionTypeTimestamps:
const expectedLength = 10
if length != expectedLength {
return options{}, fmt.Errorf("%w: timestamps option at offset %d has length %d, expected %d",
ErrTCPOptionTimestampsInvalid, i, length, expectedLength)
return options{}, fmt.Errorf("TCP option Timestamps value is invalid: "+
"timestamps option at offset %d has length %d, expected %d", i, length, expectedLength)
}
parsed.timestamps = &optionTimestamps{
value: binary.BigEndian.Uint32(data[:4]),
echo: binary.BigEndian.Uint32(data[4:]),
}
default:
return options{}, fmt.Errorf("%w: type %d", errTCPOptionTypeUnknown, optionType)
return options{}, fmt.Errorf("TCP option type is unknown: type %d", optionType)
}
i += length