Return error in InitConfig instead of exiting outright if something goes wrong

cmd/gochan-migration/main.go

@@ -45,9 +45,11 @@ func main() {
 	flag.StringVar(&options.OldChanConfig, "oldconfig", "", "The path to the old chan's configuration file")
 	flag.Parse()
 
-	config.InitConfig(versionStr)
-	err := common.InitMigrationLog()
+	err := config.InitConfig(versionStr)
 	if err != nil {
+		log.Fatalln("Unable to initialize configuration:", err.Error())
+	}
+	if err = common.InitMigrationLog(); err != nil {
 		log.Fatalln("Unable to initialize migration log:", err.Error())
 	}
 	fatalEv := common.LogFatal()
@@ -94,7 +96,9 @@ func main() {
 		Bool("migratingInPlace", migratingInPlace).
 		Msg("Starting database migration")
 
-	config.InitConfig(versionStr)
+	if err = config.InitConfig(versionStr); err != nil {
+		fatalEv.Err(err).Caller().Msg("Unable to reload configuration")
+	}
 	sqlCfg := config.GetSQLConfig()
 	if migratingInPlace && sqlCfg.DBtype == "sqlite3" && !updateDB {
 		common.LogWarning().

cmd/gochan/main.go

@@ -37,12 +37,21 @@ func cleanup() {
 
 func main() {
 	fmt.Printf("Starting gochan v%s\n", versionStr)
-	config.InitConfig(versionStr)
+	err := config.InitConfig(versionStr)
+	if err != nil {
+		jsonLocation := config.JSONLocation()
+		if jsonLocation != "" {
+			fmt.Printf("Failed to load configuration from %q: %s\n", jsonLocation, err.Error())
+		} else {
+			fmt.Printf("Failed to load configuration: %s\n", err.Error())
+		}
+		cleanup()
+		os.Exit(1)
+	}
 
 	uid, gid := config.GetUser()
 	systemCritical := config.GetSystemCriticalConfig()
-	err := gcutil.InitLogs(systemCritical.LogDir, true, uid, gid)
-	if err != nil {
+	if err = gcutil.InitLogs(systemCritical.LogDir, true, uid, gid); err != nil {
 		fmt.Println("Error opening logs:", err.Error())
 		cleanup()
 		os.Exit(1)

pkg/config/config.go

@@ -8,6 +8,7 @@ import (
 	"os/exec"
 	"path"
 	"strings"
+	"testing"
 
 	"github.com/Eggbertx/durationutil"
 	"github.com/gochan-org/gochan/pkg/gcutil"
@@ -35,25 +36,38 @@ type GochanConfig struct {
 	SiteConfig
 	BoardConfig
 	jsonLocation string
-	testing      bool
 }
 
-// ValidateValues checks to make sure that the configuration options are usable
-// (e.g., ListenAddress is a valid IP address, Port isn't a negative number, etc)
-func (gcfg *GochanConfig) ValidateValues() error {
-	changed := false
+// JSONLocation returns the path to the configuration file, if loaded
+func JSONLocation() string {
+	if cfg == nil {
+		return ""
+	}
+	return cfg.jsonLocation
+}
 
+func (gcfg *GochanConfig) updateDeprecatedFields() (changed bool) {
 	if gcfg.ListenIP != "" && gcfg.ListenAddress == "" {
 		gcfg.ListenAddress = gcfg.ListenIP
 		gcfg.ListenIP = ""
 		changed = true
 	}
-
 	if gcfg.SiteDomain != "" && gcfg.SiteHost == "" {
 		gcfg.SiteHost = gcfg.SiteDomain
 		gcfg.SiteDomain = ""
 		changed = true
 	}
+	if gcfg.NewTabOnOutlinks && !gcfg.NewTabOnExternalLinks {
+		gcfg.NewTabOnExternalLinks = true
+		changed = true
+	}
+	return changed
+}
+
+// ValidateValues checks to make sure that the configuration options are usable
+// (e.g., ListenAddress is a valid IP address, Port isn't a negative number, etc)
+func (gcfg *GochanConfig) ValidateValues() error {
+	changed := gcfg.updateDeprecatedFields()
 
 	if gcfg.SiteHost == "" {
 		return &InvalidValueError{Field: "SiteHost", Value: gcfg.SiteHost, Details: "must be set"}
@@ -78,6 +92,7 @@ func (gcfg *GochanConfig) ValidateValues() error {
 
 	if gcfg.DBtype == "postgresql" {
 		gcfg.DBtype = "postgres"
+		changed = true
 	}
 	found := false
 	drivers := sql.Drivers()
@@ -136,7 +151,7 @@ func (gcfg *GochanConfig) Write() error {
 	if err != nil {
 		return err
 	}
-	if gcfg.testing {
+	if testing.Testing() {
 		// don't try to write anything if we're doing a test
 		return nil
 	}
@@ -577,7 +592,12 @@ type PostConfig struct {
 
 	// NewTabOnExternalLinks determines whether to open external links in a new tab
 	// Default: true
-	NewTabOnExternalLinks bool `json:"NewTabOnOutlinks"`
+	NewTabOnExternalLinks bool
+
+	// NewTabOnOutlinks is an alias for the NewTabOnExternalLinks field.
+	//
+	// Deprecated: Use NewTabOnExternalLinks instead
+	NewTabOnOutlinks bool `json:",omitempty"`
 
 	// DisableBBcode will disable BBCode to HTML conversion if true
 	// Default: false

pkg/config/config_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	_ "github.com/go-sql-driver/mysql"
+	"github.com/gochan-org/gochan/pkg/gcutil/testutil"
 	_ "github.com/lib/pq"
 	_ "github.com/mattn/go-sqlite3"
 	"github.com/stretchr/testify/assert"
@@ -24,7 +25,10 @@ func TestValidJSON(t *testing.T) {
 }
 
 func TestValidateValues(t *testing.T) {
-	InitConfig("3.1.0")
+	testutil.GoToGochanRoot(t)
+	if !assert.NoError(t, InitConfig("4.1.0")) {
+		t.FailNow()
+	}
 	SetRandomSeed("test")
 	assert.NoError(t, cfg.ValidateValues())
 
@@ -46,7 +50,10 @@ type webRootTest struct {
 }
 
 func TestWebPath(t *testing.T) {
-	InitConfig("4.0.0")
+	testutil.GoToGochanRoot(t)
+	if !assert.NoError(t, InitConfig("4.1.0")) {
+		t.FailNow()
+	}
 	testCases := []webRootTest{
 		{
 			webRoot:    "/",

pkg/config/lua_test.go

@@ -3,6 +3,7 @@ package config
 import (
 	"testing"
 
+	"github.com/gochan-org/gochan/pkg/gcutil/testutil"
 	"github.com/stretchr/testify/assert"
 	lua "github.com/yuin/gopher-lua"
 )
@@ -29,7 +30,10 @@ func (tC *preloadTest) run(t *testing.T) {
 }
 
 func TestPreload(t *testing.T) {
-	InitConfig("4.0.0")
+	testutil.GoToGochanRoot(t)
+	if !assert.NoError(t, InitConfig("4.1.0")) {
+		t.FailNow()
+	}
 	testCases := []preloadTest{
 		{
 			desc: "access system critical config from lua",

pkg/config/util.go

@@ -2,6 +2,7 @@ package config
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io/fs"
 	"os"
@@ -9,7 +10,7 @@ import (
 	"path"
 	"runtime"
 	"strconv"
-	"strings"
+	"testing"
 	"time"
 
 	"github.com/gochan-org/gochan/pkg/gcutil"
@@ -71,18 +72,18 @@ func TakeOwnershipOfFile(f *os.File) error {
 	return f.Chown(uid, gid)
 }
 
-// InitConfig loads and parses gochan.json on startup and verifies its contents
-func InitConfig(versionStr string) {
+func loadConfig(versionStr string, searchPaths ...string) (err error) {
 	cfg = defaultGochanConfig
-	if strings.HasSuffix(os.Args[0], ".test") {
+	if testing.Testing() {
 		// create a dummy config for testing if we're using go test
 		cfg = defaultGochanConfig
 		cfg.ListenAddress = "127.0.0.1"
 		cfg.Port = 8080
 		cfg.UseFastCGI = true
-		cfg.testing = true
 		cfg.TemplateDir = "templates"
+		cfg.LogDir = "log"
 		cfg.DBtype = "sqlite3"
+		cfg.DocumentRoot = "html"
 		cfg.DBhost = "./testdata/gochantest.db"
 		cfg.DBname = "gochan"
 		cfg.DBusername = "gochan"
@@ -99,30 +100,40 @@ func InitConfig(versionStr string) {
 		}
 		return
 	}
-	cfgPath = gcutil.FindResource(
-		"gochan.json",
-		"/usr/local/etc/gochan/gochan.json",
-		"/etc/gochan/gochan.json")
+	cfgPath = gcutil.FindResource(searchPaths...)
 	if cfgPath == "" {
-		fmt.Println("gochan.json not found")
-		os.Exit(1)
+		return errors.New("gochan.json not found")
 	}
 
 	cfgBytes, err := os.ReadFile(cfgPath)
 	if err != nil {
-		fmt.Printf("Error reading %s: %s\n", cfgPath, err.Error())
-		os.Exit(1)
+		return fmt.Errorf("error reading %s: %w", cfgPath, err)
 	}
 
 	if err = json.Unmarshal(cfgBytes, cfg); err != nil {
-		fmt.Printf("Error parsing %s: %s", cfgPath, err.Error())
-		os.Exit(1)
+		var unmarshalTypeError *json.UnmarshalTypeError
+		if errors.As(err, &unmarshalTypeError) {
+			return fmt.Errorf("invalid field type %s in %s: expected %s, found %s",
+				unmarshalTypeError.Field, cfgPath, unmarshalTypeError.Type, unmarshalTypeError.Value)
+		}
+		return fmt.Errorf("error parsing %s: %w", cfgPath, err)
 	}
 	cfg.jsonLocation = cfgPath
+	return nil
+}
+
+// InitConfig loads and parses gochan.json on startup and verifies its contents
+func InitConfig(versionStr string) (err error) {
+	var searchPaths []string
+	if !testing.Testing() {
+		searchPaths = []string{"gochan.json", "/usr/local/etc/gochan/gochan.json", "/etc/gochan/gochan.json"}
+	}
+	if err = loadConfig(versionStr, searchPaths...); err != nil {
+		return err
+	}
 
 	if err = cfg.ValidateValues(); err != nil {
-		fmt.Println(err.Error())
-		os.Exit(1)
+		return err
 	}
 
 	if runtime.GOOS != "windows" {
@@ -133,34 +144,28 @@ func InitConfig(versionStr string) {
 			gcUser, err = user.Current()
 		}
 		if err != nil {
-			fmt.Println(err.Error())
-			os.Exit(1)
+			return err
 		}
 		if uid, err = strconv.Atoi(gcUser.Uid); err != nil {
-			fmt.Println(err.Error())
-			os.Exit(1)
+			return err
 		}
 
 		if gid, err = strconv.Atoi(gcUser.Gid); err != nil {
-			fmt.Println(err.Error())
-			os.Exit(1)
+			return err
 		}
 	}
 
 	if _, err = os.Stat(cfg.DocumentRoot); err != nil {
-		fmt.Println(err.Error())
-		os.Exit(1)
+		return err
 	}
 	if _, err = os.Stat(cfg.TemplateDir); err != nil {
-		fmt.Println(err.Error())
-		os.Exit(1)
+		return err
 	}
 	if _, err = os.Stat(cfg.LogDir); os.IsNotExist(err) {
 		err = os.MkdirAll(cfg.LogDir, DirFileMode)
 	}
 	if err != nil {
-		fmt.Println(err.Error())
-		os.Exit(1)
+		return err
 	}
 
 	cfg.LogDir = gcutil.FindResource(cfg.LogDir, "log", "/var/log/gochan/")
@@ -189,6 +194,7 @@ func InitConfig(versionStr string) {
 
 	cfg.Version = ParseVersion(versionStr)
 	cfg.Version.Normalize()
+	return nil
 }
 
 // WebPath returns an absolute path, starting at the web root (which is "/" by default)

pkg/gcplugin/lua_test.go

@@ -74,7 +74,10 @@ func TestEventModule(t *testing.T) {
 }
 
 func TestConfigModule(t *testing.T) {
-	config.InitConfig(config.GetVersion().String())
+	testutil.GoToGochanRoot(t)
+	if !assert.NoError(t, config.InitConfig("4.1.0")) {
+		t.FailNow()
+	}
 	initPluginTests()
 	err := lState.DoString(configTestingStr)
 	assert.NoError(t, err)

pkg/gcutil/testutil/testutil.go

@@ -4,7 +4,6 @@ import (
 	"errors"
 	"os"
 	"path/filepath"
-	"strings"
 	"testing"
 )
 
@@ -14,7 +13,7 @@ const (
 
 // PanicIfNotTest panics if the function was called directly or indirectly by a test function via go test
 func PanicIfNotTest() {
-	if !strings.HasSuffix(os.Args[0], ".test") && !strings.HasSuffix(os.Args[0], ".test.exe") && os.Args[1] != "-test.run" {
+	if !testing.Testing() {
 		panic("the testutil package should only be used in tests")
 	}
 }