fix: remove most uses of log.Fatal when running programatically

cmd/run.go

@@ -56,14 +56,17 @@ var runCmd = &cobra.Command{
 			excludeRE = regexp.MustCompile(exclude)
 		}
 
-		currentRun := runner.Run(tests, runner.Config{
+		currentRun, err := runner.Run(tests, runner.Config{
 			Include:        includeRE,
 			Exclude:        excludeRE,
 			ShowTime:       showTime,
 			Quiet:          quiet,
 			ConnectTimeout: connectTimeout,
 			ReadTimeout:    readTimeout,
 		})
+		if err != nil {
+			log.Fatal().Err(err)
+		}
 
 		os.Exit(currentRun.Stats.TotalFailed())
 	},

ftwhttp/client.go

@@ -21,17 +21,17 @@ func NewClientConfig() ClientConfig {
 }
 
 // NewClient initializes the http client, creating the cookiejar
-func NewClient(config ClientConfig) *Client {
+func NewClient(config ClientConfig) (*Client, error) {
 	// All users of cookiejar should import "golang.org/x/net/publicsuffix"
 	jar, err := cookiejar.New(&cookiejar.Options{PublicSuffixList: publicsuffix.List})
 	if err != nil {
-		log.Fatal().Err(err)
+		return nil, err
 	}
 	c := &Client{
 		Jar:    jar,
 		config: config,
 	}
-	return c
+	return c, nil
 }
 
 // NewConnection creates a new Connection based on a Destination

ftwhttp/client_test.go

@@ -7,7 +7,8 @@ import (
 )
 
 func TestNewClient(t *testing.T) {
-	c := NewClient(NewClientConfig())
+	c, err := NewClient(NewClientConfig())
+	assert.NoError(t, err)
 
 	assert.NotNil(t, c.Jar, "Error creating Client")
 }
@@ -19,9 +20,10 @@ func TestConnectDestinationHTTPS(t *testing.T) {
 		Protocol: "https",
 	}
 
-	c := NewClient(NewClientConfig())
+	c, err := NewClient(NewClientConfig())
+	assert.NoError(t, err)
 
-	err := c.NewConnection(*d)
+	err = c.NewConnection(*d)
 	assert.NoError(t, err, "This should not error")
 	assert.Equal(t, "https", c.Transport.protocol, "Error connecting to example.com using https")
 }
@@ -33,11 +35,12 @@ func TestDoRequest(t *testing.T) {
 		Protocol: "https",
 	}
 
-	c := NewClient(NewClientConfig())
+	c, err := NewClient(NewClientConfig())
+	assert.NoError(t, err)
 
 	req := generateBaseRequestForTesting()
 
-	err := c.NewConnection(*d)
+	err = c.NewConnection(*d)
 	assert.NoError(t, err, "This should not error")
 
 	_, err = c.Do(*req)
@@ -52,7 +55,8 @@ func TestGetTrackedTime(t *testing.T) {
 		Protocol: "https",
 	}
 
-	c := NewClient(NewClientConfig())
+	c, err := NewClient(NewClientConfig())
+	assert.NoError(t, err)
 
 	rl := &RequestLine{
 		Method:  "POST",
@@ -65,7 +69,7 @@ func TestGetTrackedTime(t *testing.T) {
 	data := []byte(`test=me&one=two&one=twice`)
 	req := NewRequest(rl, h, data, true)
 
-	err := c.NewConnection(*d)
+	err = c.NewConnection(*d)
 	assert.NoError(t, err, "This should not error")
 
 	c.StartTrackingTime()
@@ -90,7 +94,8 @@ func TestClientMultipartFormDataRequest(t *testing.T) {
 		Protocol: "https",
 	}
 
-	c := NewClient(NewClientConfig())
+	c, err := NewClient(NewClientConfig())
+	assert.NoError(t, err)
 
 	rl := &RequestLine{
 		Method:  "POST",
@@ -112,7 +117,7 @@ Some-file-test-here
 
 	req := NewRequest(rl, h, data, true)
 
-	err := c.NewConnection(*d)
+	err = c.NewConnection(*d)
 	assert.NoError(t, err, "This should not error")
 
 	c.StartTrackingTime()
@@ -127,7 +132,8 @@ Some-file-test-here
 }
 
 func TestNewConnectionCreatesTransport(t *testing.T) {
-	c := NewClient(NewClientConfig())
+	c, err := NewClient(NewClientConfig())
+	assert.NoError(t, err)
 	assert.Nil(t, c.Transport, "Transport not expected to initialized yet")
 
 	server := testServer()
@@ -141,7 +147,8 @@ func TestNewConnectionCreatesTransport(t *testing.T) {
 }
 
 func TestNewOrReusedConnectionCreatesTransport(t *testing.T) {
-	c := NewClient(NewClientConfig())
+	c, err := NewClient(NewClientConfig())
+	assert.NoError(t, err)
 	assert.Nil(t, c.Transport, "Transport not expected to initialized yet")
 
 	server := testServer()
@@ -155,7 +162,8 @@ func TestNewOrReusedConnectionCreatesTransport(t *testing.T) {
 }
 
 func TestNewOrReusedConnectionReusesTransport(t *testing.T) {
-	c := NewClient(NewClientConfig())
+	c, err := NewClient(NewClientConfig())
+	assert.NoError(t, err)
 	assert.Nil(t, c.Transport, "Transport not expected to initialized yet")
 
 	server := testServer()

ftwhttp/connection.go

@@ -5,6 +5,7 @@ import (
 	"bufio"
 	"bytes"
 	"errors"
+	"fmt"
 	"io"
 	"net"
 	"net/http"
@@ -82,7 +83,7 @@ func (c *Connection) Request(request *Request) error {
 	// Build request first, then connect and send, so timers are accurate
 	data, err := buildRequest(request)
 	if err != nil {
-		log.Fatal().Msgf("ftw/http: fatal error building request: %s", err.Error())
+		return fmt.Errorf("ftw/http: fatal error building request: %w", err)
 	}
 
 	log.Debug().Msgf("ftw/http: sending data:\n%s\n", data)

ftwhttp/response_test.go

@@ -88,7 +88,8 @@ func TestResponse(t *testing.T) {
 
 	req := generateRequestForTesting(true)
 
-	client := NewClient(NewClientConfig())
+	client, err := NewClient(NewClientConfig())
+	assert.NoError(t, err)
 	err = client.NewConnection(*d)
 	assert.NoError(t, err)
 
@@ -107,7 +108,8 @@ func TestResponseWithCookies(t *testing.T) {
 	assert.NoError(t, err)
 	req := generateRequestForTesting(true)
 
-	client := NewClient(NewClientConfig())
+	client, err := NewClient(NewClientConfig())
+	assert.NoError(t, err)
 	err = client.NewConnection(*d)
 
 	assert.NoError(t, err)

runner/run.go

@@ -1,6 +1,7 @@
 package runner
 
 import (
+	"errors"
 	"fmt"
 	"regexp"
 	"time"
@@ -17,11 +18,16 @@ import (
 	"github.com/coreruleset/go-ftw/waflog"
 )
 
+var errBadTestRequest = errors.New("ftw/run: bad test: choose between data, encoded_request, or raw_request")
+
 // Run runs your tests with the specified Config. Returns error if some test failed
-func Run(tests []test.FTWTest, c Config) TestRunContext {
+func Run(tests []test.FTWTest, c Config) (TestRunContext, error) {
 	printUnlessQuietMode(c.Quiet, ":rocket:Running go-ftw!\n")
 
-	logLines := waflog.NewFTWLogLines(waflog.WithLogFile(config.FTWConfig.LogFile))
+	logLines, err := waflog.NewFTWLogLines(waflog.WithLogFile(config.FTWConfig.LogFile))
+	if err != nil {
+		return TestRunContext{}, err
+	}
 
 	conf := ftwhttp.NewClientConfig()
 	if c.ConnectTimeout != 0 {
@@ -30,7 +36,10 @@ func Run(tests []test.FTWTest, c Config) TestRunContext {
 	if c.ReadTimeout != 0 {
 		conf.ReadTimeout = c.ReadTimeout
 	}
-	client := ftwhttp.NewClient(conf)
+	client, err := ftwhttp.NewClient(conf)
+	if err != nil {
+		return TestRunContext{}, err
+	}
 	runContext := TestRunContext{
 		Include:  c.Include,
 		Exclude:  c.Exclude,
@@ -41,21 +50,23 @@ func Run(tests []test.FTWTest, c Config) TestRunContext {
 		RunMode:  config.FTWConfig.RunMode,
 	}
 
-	for _, test := range tests {
-		RunTest(&runContext, test)
+	for _, tc := range tests {
+		if err := RunTest(&runContext, tc); err != nil {
+			return TestRunContext{}, err
+		}
 	}
 
 	printSummary(c.Quiet, runContext.Stats)
 
 	defer cleanLogs(logLines)
 
-	return runContext
+	return runContext, nil
 }
 
 // RunTest runs an individual test.
 // runContext contains information for the current test run
 // ftwTest is the test you want to run
-func RunTest(runContext *TestRunContext, ftwTest test.FTWTest) {
+func RunTest(runContext *TestRunContext, ftwTest test.FTWTest) error {
 	changed := true
 
 	for _, testCase := range ftwTest.Tests {
@@ -78,17 +89,21 @@ func RunTest(runContext *TestRunContext, ftwTest test.FTWTest) {
 		// Iterate over stages
 		for _, stage := range testCase.Stages {
 			ftwCheck := check.NewCheck(config.FTWConfig)
-			RunStage(runContext, ftwCheck, testCase, stage.Stage)
+			if err := RunStage(runContext, ftwCheck, testCase, stage.Stage); err != nil {
+				return err
+			}
 		}
 	}
+
+	return nil
 }
 
 // RunStage runs an individual test stage.
 // runContext contains information for the current test run
 // ftwCheck is the current check utility
 // testCase is the test case the stage belongs to
 // stage is the stage you want to run
-func RunStage(runContext *TestRunContext, ftwCheck *check.FTWCheck, testCase test.Test, stage test.Stage) {
+func RunStage(runContext *TestRunContext, ftwCheck *check.FTWCheck, testCase test.Test, stage test.Stage) error {
 	stageStartTime := time.Now()
 	stageID := uuid.NewString()
 	// Apply global overrides initially
@@ -101,14 +116,14 @@ func RunStage(runContext *TestRunContext, ftwCheck *check.FTWCheck, testCase tes
 
 	// Check sanity first
 	if checkTestSanity(testRequest) {
-		log.Fatal().Msgf("ftw/run: bad test: choose between data, encoded_request, or raw_request")
+		return errBadTestRequest
 	}
 
 	// Do not even run test if result is overridden. Just use the override and display the overridden result.
 	if overridden := overriddenTestResult(ftwCheck, testCase.TestTitle); overridden != Failed {
 		addResultToStats(overridden, testCase.TestTitle, &runContext.Stats)
 		displayResult(runContext.Output, overridden, time.Duration(0), time.Duration(0))
-		return
+		return nil
 	}
 
 	var req *ftwhttp.Request
@@ -123,7 +138,7 @@ func RunStage(runContext *TestRunContext, ftwCheck *check.FTWCheck, testCase tes
 	if notRunningInCloudMode(ftwCheck) {
 		startMarker, err := markAndFlush(runContext, dest, stageID)
 		if err != nil && !expectedOutput.ExpectError {
-			log.Fatal().Caller().Err(err).Msg("Failed to find start marker")
+			return fmt.Errorf("failed to find start marker: %w", err)
 		}
 		ftwCheck.SetStartMarker(startMarker)
 	}
@@ -133,21 +148,21 @@ func RunStage(runContext *TestRunContext, ftwCheck *check.FTWCheck, testCase tes
 	err = runContext.Client.NewConnection(*dest)
 
 	if err != nil && !expectedOutput.ExpectError {
-		log.Fatal().Caller().Err(err).Msgf("can't connect to destination %+v", dest)
+		return fmt.Errorf("can't connect to destination %+v: %w", dest, err)
 	}
 	runContext.Client.StartTrackingTime()
 
 	response, responseErr := runContext.Client.Do(*req)
 
 	runContext.Client.StopTrackingTime()
 	if responseErr != nil && !expectedOutput.ExpectError {
-		log.Fatal().Caller().Err(responseErr).Msgf("failed sending request to destination %+v", dest)
+		return fmt.Errorf("failed sending request to destination %+v: %w", dest, responseErr)
 	}
 
 	if notRunningInCloudMode(ftwCheck) {
 		endMarker, err := markAndFlush(runContext, dest, stageID)
 		if err != nil && !expectedOutput.ExpectError {
-			log.Fatal().Caller().Err(err).Msg("Failed to find end marker")
+			return fmt.Errorf("failed to find end marker: %w", err)
 
 		}
 		ftwCheck.SetEndMarker(endMarker)
@@ -171,6 +186,8 @@ func RunStage(runContext *TestRunContext, ftwCheck *check.FTWCheck, testCase tes
 
 	runContext.Stats.Run++
 	runContext.Stats.RunTime += stageTime
+
+	return nil
 }
 
 func markAndFlush(runContext *TestRunContext, dest *ftwhttp.Destination, stageID string) ([]byte, error) {
@@ -386,6 +403,6 @@ func notRunningInCloudMode(c *check.FTWCheck) bool {
 
 func cleanLogs(logLines *waflog.FTWLogLines) {
 	if err := logLines.Cleanup(); err != nil {
-		log.Fatal().Err(err).Msg("Failed to cleanup log file")
+		log.Error().Err(err).Msg("Failed to cleanup log file")
 	}
 }