core: don't panic, return errors instead (#183)

Author: markus-wa

datatables.go

@@ -132,7 +132,7 @@ func (p *Parser) bindTeamStates() {
 		case "Spectator": // Ignore
 
 		default:
-			panic(fmt.Sprintf("Unexpected team %q", team))
+			p.setError(fmt.Errorf("unexpected team %q", team))
 		}
 
 		if s != nil {
@@ -505,7 +505,7 @@ func (p *Parser) bindWeapon(entity *st.Entity, wepType common.EquipmentElement)
 		if strings.Contains(eq.OriginalString, altName) {
 			eq.Weapon = alt
 		} else if !strings.Contains(eq.OriginalString, defaultName) {
-			panic(fmt.Sprintf("Unknown weapon model %q", eq.OriginalString))
+			p.setError(fmt.Errorf("unknown weapon model %q", eq.OriginalString))
 		}
 	}
 

demoinfocs_test.go

@@ -100,41 +100,29 @@ func TestDemoInfoCs(t *testing.T) {
 
 	// bomb planting checks
 	p.RegisterEventHandler(func(begin events.BombPlantBegin) {
-		if !begin.Player.IsPlanting {
-			t.Error("Player started planting but IsPlanting is false")
-		}
+		assertions.True(begin.Player.IsPlanting, "Player started planting but IsPlanting is false")
 	})
 	p.RegisterEventHandler(func(abort events.BombPlantAborted) {
-		if abort.Player.IsPlanting {
-			t.Error("Player aborted planting but IsPlanting is true")
-		}
+		assertions.False(abort.Player.IsPlanting, "Player aborted planting but IsPlanting is true")
 	})
 	p.RegisterEventHandler(func(planted events.BombPlanted) {
-		if planted.Player.IsPlanting {
-			t.Error("Player finished planting but IsPlanting is true")
-		}
+		assertions.False(planted.Player.IsPlanting, "Player finished planting but IsPlanting is true")
 	})
 
 	// airborne checks
 	// we don't check RoundStart or RoundFreezetimeEnd since players may spawn airborne
 	p.RegisterEventHandler(func(plantBegin events.BombPlantBegin) {
-		if plantBegin.Player.IsAirborne() {
-			t.Error("Player is airborne during plant")
-		}
+		assertions.False(plantBegin.Player.IsAirborne(), "Player can't be airborne during plant")
 	})
 
 	// reload checks
 	p.RegisterEventHandler(func(reload events.WeaponReload) {
-		if !reload.Player.IsReloading {
-			t.Error("Player started reloading but IsReloading is false")
-		}
+		assertions.True(reload.Player.IsReloading, "Player started reloading but IsReloading is false")
 	})
 
 	p.RegisterEventHandler(func(start events.RoundFreezetimeEnd) {
 		for _, pl := range p.GameState().Participants().All() {
-			if pl.IsReloading {
-				t.Error("Player is reloading at the start of the round")
-			}
+			assertions.False(pl.IsReloading, "Player can't be reloading at the start of the round")
 		}
 	})
 

game_events.go

@@ -1,6 +1,7 @@
 package demoinfocs
 
 import (
+	"errors"
 	"fmt"
 	"strconv"
 
@@ -419,7 +420,12 @@ func (geh gameEventHandler) playerConnect(data map[string]*msg.CSVCMsg_GameEvent
 		guid:   data["networkid"].GetValString(),
 	}
 
-	pl.xuid = getCommunityID(pl.guid)
+	var err error
+	pl.xuid, err = getCommunityID(pl.guid)
+
+	if err != nil {
+		geh.parser.setError(fmt.Errorf("failed to parse player XUID: %v", err.Error()))
+	}
 
 	geh.parser.rawPlayers[int(data["index"].GetValByte())] = pl
 }
@@ -511,7 +517,7 @@ func (geh gameEventHandler) bombEvent(data map[string]*msg.CSVCMsg_GameEventKeyT
 		t := geh.parser.triggers[site]
 
 		if t == nil {
-			panic(fmt.Sprintf("Bombsite with index %d not found", site))
+			geh.parser.setError(fmt.Errorf("bombsite with index %d not found", site))
 		}
 
 		if t.contains(geh.parser.bombsiteA.center) {
@@ -521,7 +527,7 @@ func (geh gameEventHandler) bombEvent(data map[string]*msg.CSVCMsg_GameEventKeyT
 			bombEvent.Site = events.BombsiteB
 			geh.parser.bombsiteB.index = site
 		} else {
-			panic("Bomb not planted on bombsite A or B")
+			geh.parser.setError(errors.New("bomb not planted on bombsite A or B"))
 		}
 	}
 
@@ -703,21 +709,21 @@ func mapGameEventData(d *msg.CSVCMsg_GameEventListDescriptorT, e *msg.CSVCMsg_Ga
 // We're all better off not asking questions
 const valveMagicNumber = 76561197960265728
 
-func getCommunityID(guid string) int64 {
+func getCommunityID(guid string) (int64, error) {
 	if guid == "BOT" {
-		return 0
+		return 0, nil
 	}
 
 	authSrv, errSrv := strconv.ParseInt(guid[8:9], 10, 64)
 	authID, errID := strconv.ParseInt(guid[10:], 10, 64)
 
 	if errSrv != nil {
-		panic(errSrv.Error())
+		return 0, errSrv
 	}
 	if errID != nil {
-		panic(errID.Error())
+		return 0, errID
 	}
 
 	// WTF are we doing here?
-	return valveMagicNumber + authID*2 + authSrv
+	return valveMagicNumber + authID*2 + authSrv, nil
 }

net_messages.go

@@ -12,7 +12,7 @@ const entitySentinel = 9999
 
 func (p *Parser) handlePacketEntities(pe *msg.CSVCMsg_PacketEntities) {
 	defer func() {
-		p.setError(recoverFromUnexpectedEOF(recover()))
+		p.setError(recoverFromPanic(recover()))
 	}()
 
 	r := bit.NewSmallBitReader(bytes.NewReader(pe.EntityData))

parser_test.go

@@ -1,6 +1,8 @@
 package demoinfocs
 
 import (
+	"errors"
+	"io"
 	"math"
 	"testing"
 	"time"
@@ -62,3 +64,15 @@ func TestParser_Progress_NoHeader(t *testing.T) {
 	assert.Zero(t, new(Parser).Progress())
 	assert.Zero(t, (&Parser{header: &common.DemoHeader{}}).Progress())
 }
+
+func TestRecoverFromPanic(t *testing.T) {
+	assert.Nil(t, recoverFromPanic(nil))
+	assert.Equal(t, ErrUnexpectedEndOfDemo, recoverFromPanic(io.ErrUnexpectedEOF))
+	assert.Equal(t, ErrUnexpectedEndOfDemo, recoverFromPanic(io.EOF))
+
+	err := errors.New("test")
+	assert.Equal(t, err, recoverFromPanic(err))
+
+	assert.Equal(t, "test", recoverFromPanic("test").Error())
+	assert.Equal(t, "unexpected error: 1", recoverFromPanic(1).Error())
+}

parsing.go

@@ -84,7 +84,7 @@ func (p *Parser) ParseToEnd() (err error) {
 		}
 
 		if err == nil {
-			err = recoverFromUnexpectedEOF(recover())
+			err = recoverFromPanic(recover())
 		}
 	}()
 
@@ -112,14 +112,23 @@ func (p *Parser) ParseToEnd() (err error) {
 	}
 }
 
-func recoverFromUnexpectedEOF(r interface{}) error {
-	if r != nil {
-		if r == io.ErrUnexpectedEOF || r == io.EOF {
-			return ErrUnexpectedEndOfDemo
-		}
-		panic(r)
+func recoverFromPanic(r interface{}) error {
+	if r == nil {
+		return nil
+	}
+
+	if r == io.ErrUnexpectedEOF || r == io.EOF {
+		return ErrUnexpectedEndOfDemo
+	}
+
+	switch err := r.(type) {
+	case error:
+		return err
+	case string:
+		return errors.New(err)
+	default:
+		return fmt.Errorf("unexpected error: %v", err)
 	}
-	return nil
 }
 
 // Cancel aborts ParseToEnd().
@@ -149,7 +158,7 @@ func (p *Parser) ParseNextFrame() (moreFrames bool, err error) {
 		}
 
 		if err == nil {
-			err = recoverFromUnexpectedEOF(recover())
+			err = recoverFromPanic(recover())
 		}
 	}()
 

stringtables.go

@@ -99,7 +99,7 @@ func (p *Parser) parseSingleStringTable(name string) {
 }
 
 func (p *Parser) handleUpdateStringTable(tab *msg.CSVCMsg_UpdateStringTable) {
-	// No need for recoverFromUnexpectedEOF here as we do that in processStringTable already
+	// No need for recoverFromPanic here as we do that in processStringTable already
 
 	cTab := p.stringTables[tab.TableId]
 	switch cTab.Name {
@@ -117,7 +117,7 @@ func (p *Parser) handleUpdateStringTable(tab *msg.CSVCMsg_UpdateStringTable) {
 }
 
 func (p *Parser) handleCreateStringTable(tab *msg.CSVCMsg_CreateStringTable) {
-	// No need for recoverFromUnexpectedEOF here as we do that in processStringTable already
+	// No need for recoverFromPanic here as we do that in processStringTable already
 
 	p.processStringTable(tab)
 
@@ -128,7 +128,7 @@ func (p *Parser) handleCreateStringTable(tab *msg.CSVCMsg_CreateStringTable) {
 
 func (p *Parser) processStringTable(tab *msg.CSVCMsg_CreateStringTable) {
 	defer func() {
-		p.setError(recoverFromUnexpectedEOF(recover()))
+		p.setError(recoverFromPanic(recover()))
 	}()
 
 	if tab.Name == stNameModelPreCache {