Don't panic or log fatal in the library code (#16)

In order to accommodate forward and backward compatibility in the parser, parsing errors cannot be fatal or interrupt the main program. Instead, throw them in an error channel and let the client code decide what to do. Many parsing errors, like missing or extra fields, are not going to be reason to stop listening.

README.md

@@ -1,6 +1,6 @@
-[![PkgGoDev](https://pkg.go.dev/badge/github.com/k0swe/wsjtx-go)](https://pkg.go.dev/github.com/k0swe/wsjtx-go)
-[![Go Report Card](https://goreportcard.com/badge/github.com/k0swe/wsjtx-go)](https://goreportcard.com/report/github.com/k0swe/wsjtx-go)
-[![Test](https://github.com/k0swe/wsjtx-go/workflows/Test/badge.svg?branch=main)](https://github.com/k0swe/wsjtx-go/actions/workflows/test.yml?query=branch%3Amain)
+[![PkgGoDev](https://pkg.go.dev/badge/github.com/k0swe/wsjtx-go/v2)](https://pkg.go.dev/github.com/k0swe/wsjtx-go/v2)
+[![Go Report Card](https://goreportcard.com/badge/github.com/k0swe/wsjtx-go/v2)](https://goreportcard.com/report/github.com/k0swe/wsjtx-go/v2)
+[![Test](https://github.com/k0swe/wsjtx-go/workflows/Test/badge.svg?branch=v2)](https://github.com/k0swe/wsjtx-go/actions/workflows/test.yml?query=branch%3Av2)
 
 # wsjtx-go
 

cmd/main.go

@@ -2,25 +2,31 @@ package main
 
 import (
 	"bufio"
-	"fmt"
-	"github.com/k0swe/wsjtx-go"
+	"github.com/k0swe/wsjtx-go/v2"
+	"log"
 	"os"
 	"reflect"
 	"strings"
 )
 
 // Simple driver binary for wsjtx-go library.
 func main() {
-	fmt.Println("Listening for WSJT-X...")
+	log.Println("Listening for WSJT-X...")
+	wsjtxServer, err := wsjtx.MakeServer()
+	if err != nil {
+		log.Fatalf("%v", err)
+	}
 	wsjtxChannel := make(chan interface{}, 5)
-	wsjtxServer := wsjtx.MakeServer()
-	go wsjtxServer.ListenToWsjtx(wsjtxChannel)
+	errChannel := make(chan error, 5)
+	go wsjtxServer.ListenToWsjtx(wsjtxChannel, errChannel)
 
 	stdinChannel := make(chan string, 5)
 	go stdinCmd(stdinChannel)
 
 	for {
 		select {
+		case err := <-errChannel:
+			log.Printf("error: %v", err)
 		case message := <-wsjtxChannel:
 			handleServerMessage(message)
 		case command := <-stdinChannel:
@@ -39,7 +45,7 @@ func stdinCmd(c chan string) {
 			c <- input
 		}
 		if err := scanner.Err(); err != nil {
-			fmt.Println(err)
+			log.Println(err)
 			os.Exit(1)
 		}
 	}
@@ -49,23 +55,23 @@ func stdinCmd(c chan string) {
 func handleServerMessage(message interface{}) {
 	switch message.(type) {
 	case wsjtx.HeartbeatMessage:
-		fmt.Println("Heartbeat:", message)
+		log.Println("Heartbeat:", message)
 	case wsjtx.StatusMessage:
-		fmt.Println("Status:", message)
+		log.Println("Status:", message)
 	case wsjtx.DecodeMessage:
-		fmt.Println("Decode:", message)
+		log.Println("Decode:", message)
 	case wsjtx.ClearMessage:
-		fmt.Println("Clear:", message)
+		log.Println("Clear:", message)
 	case wsjtx.QsoLoggedMessage:
-		fmt.Println("QSO Logged:", message)
+		log.Println("QSO Logged:", message)
 	case wsjtx.CloseMessage:
-		fmt.Println("Close:", message)
+		log.Println("Close:", message)
 	case wsjtx.WSPRDecodeMessage:
-		fmt.Println("WSPR Decode:", message)
+		log.Println("WSPR Decode:", message)
 	case wsjtx.LoggedAdifMessage:
-		fmt.Println("Logged Adif:", message)
+		log.Println("Logged Adif:", message)
 	default:
-		fmt.Println("Other:", reflect.TypeOf(message), message)
+		log.Println("Other:", reflect.TypeOf(message), message)
 	}
 }
 
@@ -75,7 +81,7 @@ func handleCommand(command string, wsjtxServer wsjtx.Server) {
 	switch command {
 
 	case "hb":
-		fmt.Println("Sending Heartbeat")
+		log.Println("Sending Heartbeat")
 		err = wsjtxServer.Heartbeat(wsjtx.HeartbeatMessage{
 			Id:        "wsjtx-go",
 			MaxSchema: 2,
@@ -84,19 +90,19 @@ func handleCommand(command string, wsjtxServer wsjtx.Server) {
 		})
 
 	case "clear":
-		fmt.Println("Sending Clear")
+		log.Println("Sending Clear")
 		err = wsjtxServer.Clear(wsjtx.ClearMessage{Id: "WSJT-X", Window: 2})
 
 	case "close":
-		fmt.Println("Sending Close")
+		log.Println("Sending Close")
 		err = wsjtxServer.Close(wsjtx.CloseMessage{Id: "WSJT-X"})
 
 	case "replay":
-		fmt.Println("Sending Replay")
+		log.Println("Sending Replay")
 		err = wsjtxServer.Replay(wsjtx.ReplayMessage{Id: "WSJT-X"})
 
 	}
 	if err != nil {
-		fmt.Println(err)
+		log.Println(err)
 	}
 }

go.mod

@@ -1,4 +1,4 @@
-module github.com/k0swe/wsjtx-go
+module github.com/k0swe/wsjtx-go/v2
 
 go 1.14
 

parser.go

@@ -2,8 +2,8 @@ package wsjtx
 
 import (
 	"encoding/binary"
+	"fmt"
 	"github.com/leemcloughlin/jdn"
-	"log"
 	"math"
 	"reflect"
 	"time"
@@ -19,62 +19,65 @@ type parser struct {
 // https://sourceforge.net/p/wsjt/wsjtx/ci/master/tree/Network/NetworkMessage.hpp. This only parses
 // "Out" or "In/Out" message types and does not include "In" types because they will never be
 // received by WSJT-X.
-func parseMessage(buffer []byte, length int) interface{} {
+func parseMessage(buffer []byte, length int) (interface{}, error) {
 	p := parser{buffer: buffer, length: length, cursor: 0}
 	m := p.parseUint32()
 	if m != magic {
-		// Packet is not speaking the WSJT-X protocol
-		return nil
+		return nil, fmt.Errorf("packet is not speaking the WSJT-X protocol")
 	}
 	sch := p.parseUint32()
 	if sch != schema {
-		log.Println("Got a schema version I wasn't expecting:", sch)
+		return nil, fmt.Errorf("got a schema version I wasn't expecting: %d", sch)
 	}
 
 	messageType := p.parseUint32()
 	switch messageType {
 	case heartbeatNum:
 		heartbeat := p.parseHeartbeat()
-		p.checkParse(heartbeat)
-		return heartbeat
+		err := p.checkParse(heartbeat)
+		return heartbeat, err
 	case statusNum:
 		status := p.parseStatus()
-		p.checkParse(status)
-		return status
+		err := p.checkParse(status)
+		return status, err
 	case decodeNum:
 		decode := p.parseDecode()
-		p.checkParse(decode)
-		return decode
+		err := p.checkParse(decode)
+		return decode, err
 	case clearNum:
 		clear := p.parseClear()
-		p.checkParse(clear)
-		return clear
+		err := p.checkParse(clear)
+		return clear, err
 	case qsoLoggedNum:
-		qsoLogged := p.parseQsoLogged()
-		p.checkParse(qsoLogged)
-		return qsoLogged
+		qsoLogged, err := p.parseQsoLogged()
+		if err != nil {
+			return qsoLogged, err
+		}
+		err = p.checkParse(qsoLogged)
+		return qsoLogged, err
 	case closeNum:
 		closeMsg := p.parseClose()
-		p.checkParse(closeMsg)
-		return closeMsg
+		err := p.checkParse(closeMsg)
+		return closeMsg, err
 	case wsprDecodeNum:
 		wspr := p.parseWsprDecode()
-		p.checkParse(wspr)
-		return wspr
+		err := p.checkParse(wspr)
+		return wspr, err
 	case loggedAdifNum:
 		loggedAdif := p.parseLoggedAdif()
-		p.checkParse(loggedAdif)
-		return loggedAdif
+		err := p.checkParse(loggedAdif)
+		return loggedAdif, err
 	}
-	return nil
+	return nil, nil
 }
 
 // Quick sanity check that we parsed all of the message bytes
-func (p *parser) checkParse(message interface{}) {
+func (p *parser) checkParse(message interface{}) error {
 	if p.cursor != p.length {
-		log.Fatalf("Parsing WSJT-X %s: There were %d bytes left over\n",
+		return fmt.Errorf("parsing WSJT-X %s: there were %d bytes left over",
 			reflect.TypeOf(message).Name(), p.length-p.cursor)
 	}
+	return nil
 }
 
 func (p *parser) parseHeartbeat() HeartbeatMessage {
@@ -169,9 +172,13 @@ func (p *parser) parseClear() ClearMessage {
 	}
 }
 
-func (p *parser) parseQsoLogged() QsoLoggedMessage {
+func (p *parser) parseQsoLogged() (QsoLoggedMessage, error) {
+	var empty QsoLoggedMessage
 	id := p.parseUtf8()
-	timeOff := p.parseQDateTime()
+	timeOff, err := p.parseQDateTime()
+	if err != nil {
+		return empty, err
+	}
 	dxCall := p.parseUtf8()
 	dxGrid := p.parseUtf8()
 	txFrequency := p.parseUint64()
@@ -181,7 +188,10 @@ func (p *parser) parseQsoLogged() QsoLoggedMessage {
 	txPower := p.parseUtf8()
 	comments := p.parseUtf8()
 	name := p.parseUtf8()
-	timeOn := p.parseQDateTime()
+	timeOn, err := p.parseQDateTime()
+	if err != nil {
+		return empty, err
+	}
 	operatorCall := p.parseUtf8()
 	myCall := p.parseUtf8()
 	myGrid := p.parseUtf8()
@@ -205,7 +215,7 @@ func (p *parser) parseQsoLogged() QsoLoggedMessage {
 		MyGrid:           myGrid,
 		ExchangeSent:     exchangeSent,
 		ExchangeReceived: exchangeReceived,
-	}
+	}, nil
 }
 
 func (p *parser) parseClose() interface{} {
@@ -299,7 +309,7 @@ func (p *parser) parseBool() bool {
 	return value
 }
 
-func (p *parser) parseQDateTime() time.Time {
+func (p *parser) parseQDateTime() (time.Time, error) {
 	julianDay := p.parseUint64()
 	year, month, day := jdn.FromNumber(int(julianDay))
 	msSinceMidnight := int(p.parseUint32())
@@ -318,7 +328,7 @@ func (p *parser) parseQDateTime() time.Time {
 		// UTC
 		value = time.Date(year, month, day, hour, minute, second, 0, time.UTC)
 	default:
-		log.Fatalln("WSJT-X parser: Got a timespec I wasn't expecting,", timespec)
+		return value, fmt.Errorf("got a timespec I wasn't expecting: %d", timespec)
 	}
-	return value
+	return value, nil
 }

parser_test.go

@@ -141,7 +141,11 @@ func TestParseMessage(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := parseMessage(tt.args.buffer, tt.args.length); !reflect.DeepEqual(got, tt.want) {
+			got, err := parseMessage(tt.args.buffer, tt.args.length)
+			if err != nil {
+				t.Error(err)
+			}
+			if !reflect.DeepEqual(got, tt.want) {
 				t.Errorf("parseMessage() = %v, want %v", got, tt.want)
 			}
 		})