[bugfix] Return useful err on server start failure (#3879)

* [bugfix] Return useful err on `server start` failure

* remove scheduler started func

* remove tryUntil

cmd/gotosocial/action/admin/account/account.go

@@ -38,7 +38,9 @@
 func initState(ctx context.Context) (*state.State, error) {
 	var state state.State
 	state.Caches.Init()
-	state.Caches.Start()
+	if err := state.Caches.Start(); err != nil {
+		return nil, fmt.Errorf("error starting caches: %w", err)
+	}
 
 	// Only set state DB connection.
 	// Don't need Actions or Workers for this (yet).

cmd/gotosocial/action/admin/media/list.go

@@ -125,7 +125,9 @@ func setupList(ctx context.Context) (*list, error) {
 	}
 
 	state.Caches.Init()
-	state.Caches.Start()
+	if err := state.Caches.Start(); err != nil {
+		return nil, fmt.Errorf("error starting caches: %w", err)
+	}
 
 	// Only set state DB connection.
 	// Don't need Actions or Workers for this.

cmd/gotosocial/action/admin/media/prune/common.go

@@ -42,7 +42,9 @@ func setupPrune(ctx context.Context) (*prune, error) {
 	var state state.State
 
 	state.Caches.Init()
-	state.Caches.Start()
+	if err := state.Caches.Start(); err != nil {
+		return nil, fmt.Errorf("error starting caches: %w", err)
+	}
 
 	// Scheduler is required for the
 	// cleaner, but no other workers

cmd/gotosocial/action/server/server.go

@@ -22,6 +22,7 @@
 	"errors"
 	"fmt"
 	"net/http"
+	"net/netip"
 	"os"
 	"os/signal"
 	"runtime"
@@ -117,8 +118,9 @@
 	)
 
 	defer func() {
-		// Stop caches with
+		// Stop any started caches.
-		// background tasks.
+		//
+		// Noop if never started.
 		state.Caches.Stop()
 
 		if route != nil {
@@ -133,6 +135,8 @@
 		// Stop any currently running
 		// worker processes / scheduled
 		// tasks from being executed.
+		//
+		// Noop on unstarted workers.
 		state.Workers.Stop()
 
 		if state.Timelines.Home != nil {
@@ -202,7 +206,9 @@
 
 	// Initialize caches
 	state.Caches.Init()
-	state.Caches.Start()
+	if err := state.Caches.Start(); err != nil {
+		return fmt.Errorf("error starting caches: %w", err)
+	}
 
 	// Open connection to the database now caches started.
 	dbService, err := bundb.NewBunDBService(ctx, state)
@@ -240,10 +246,17 @@
 		return fmt.Errorf("error opening storage backend: %w", err)
 	}
 
+	// Parse http client allow
+	// and block range exceptions.
+	ranges, err := parseClientRanges()
+	if err != nil {
+		return err
+	}
+
 	// Prepare wrapped httpclient with config.
 	client := httpclient.New(httpclient.Config{
-		AllowRanges:           config.MustParseIPPrefixes(config.GetHTTPClientAllowIPs()),
+		AllowRanges:           ranges.allow,
-		BlockRanges:           config.MustParseIPPrefixes(config.GetHTTPClientBlockIPs()),
+		BlockRanges:           ranges.block,
 		Timeout:               config.GetHTTPClientTimeout(),
 		TLSInsecureSkipVerify: config.GetHTTPClientTLSInsecureSkipVerify(),
 	})
@@ -617,3 +630,44 @@ func compileWASM(ctx context.Context) error {
 
 	return nil
 }
+
+func parseClientRanges() (
+	*struct {
+		allow []netip.Prefix
+		block []netip.Prefix
+	},
+	error,
+) {
+	parseF := func(ips []string, ranges []netip.Prefix, flag string) error {
+		for i, ip := range ips {
+			p, err := netip.ParsePrefix(ip)
+			if err != nil {
+				return fmt.Errorf("error parsing %s value %s: %w", flag, ip, err)
+			}
+			ranges[i] = p
+		}
+		return nil
+	}
+
+	allowIPs := config.GetHTTPClientAllowIPs()
+	allowRanges := make([]netip.Prefix, len(allowIPs))
+	allowFlag := config.HTTPClientAllowIPsFlag()
+	if err := parseF(allowIPs, allowRanges, allowFlag); err != nil {
+		return nil, err
+	}
+
+	blockIPs := config.GetHTTPClientBlockIPs()
+	blockRanges := make([]netip.Prefix, len(blockIPs))
+	blockFlag := config.HTTPClientBlockIPsFlag()
+	if err := parseF(blockIPs, blockRanges, blockFlag); err != nil {
+		return nil, err
+	}
+
+	return &struct {
+		allow []netip.Prefix
+		block []netip.Prefix
+	}{
+		allow: allowRanges,
+		block: blockRanges,
+	}, nil
+}

internal/api/client/lists/lists_test.go

@@ -76,7 +76,9 @@ func (suite *ListsStandardTestSuite) SetupSuite() {
 
 func (suite *ListsStandardTestSuite) SetupTest() {
 	suite.state.Caches.Init()
-	suite.state.Caches.Start()
+	if err := suite.state.Caches.Start(); err != nil {
+		panic("error starting caches: " + err.Error())
+	}
 	testrig.StartNoopWorkers(&suite.state)
 
 	testrig.InitTestConfig()

internal/cache/cache.go

@@ -23,6 +23,7 @@
 	"codeberg.org/gruf/go-cache/v3/ttl"
 	"github.com/superseriousbusiness/gotosocial/internal/cache/headerfilter"
 	"github.com/superseriousbusiness/gotosocial/internal/config"
+	"github.com/superseriousbusiness/gotosocial/internal/gtserror"
 	"github.com/superseriousbusiness/gotosocial/internal/log"
 )
 
@@ -124,16 +125,18 @@ func (c *Caches) Init() {
 
 // Start will start any caches that require a background
 // routine, which usually means any kind of TTL caches.
-func (c *Caches) Start() {
+func (c *Caches) Start() error {
 	log.Infof(nil, "start: %p", c)
 
-	tryUntil("starting webfinger cache", 5, func() bool {
+	if !c.Webfinger.Start(5 * time.Minute) {
-		return c.Webfinger.Start(5 * time.Minute)
+		return gtserror.New("could not start webfinger cache")
-	})
+	}
 
-	tryUntil("starting statusesFilterableFields cache", 5, func() bool {
+	if !c.StatusesFilterableFields.Start(5 * time.Minute) {
-		return c.StatusesFilterableFields.Start(5 * time.Minute)
+		return gtserror.New("could not start statusesFilterableFields cache")
-	})
+	}
+
+	return nil
 }
 
 // Stop will stop any caches that require a background
@@ -141,8 +144,8 @@ func (c *Caches) Start() {
 func (c *Caches) Stop() {
 	log.Infof(nil, "stop: %p", c)
 
-	tryUntil("stopping webfinger cache", 5, c.Webfinger.Stop)
+	_ = c.Webfinger.Stop()
-	tryUntil("stopping statusesFilterableFields cache", 5, c.StatusesFilterableFields.Stop)
+	_ = c.StatusesFilterableFields.Stop()
 }
 
 // Sweep will sweep all the available caches to ensure none

internal/cache/util.go

@@ -19,11 +19,9 @@
 
 import (
 	"errors"
-	"time"
 
 	errorsv2 "codeberg.org/gruf/go-errors/v2"
 	"github.com/superseriousbusiness/gotosocial/internal/db"
-	"github.com/superseriousbusiness/gotosocial/internal/log"
 )
 
 // SentinelError is an error that can be returned and checked against to indicate a non-permanent
@@ -51,19 +49,3 @@ type nocopy struct{
 func (*nocopy) Lock() {}
 
 func (*nocopy) Unlock() {}
-
-// tryUntil will attempt to call 'do' for 'count' attempts, before panicking with 'msg'.
-func tryUntil(msg string, count int, do func() bool) {
-	for i := 0; i < count; i++ {
-		if do() {
-			// success.
-			return
-		}
-
-		// Sleep for a little before retry (a bcakoff).
-		time.Sleep(time.Millisecond * 1 << (i + 1))
-	}
-
-	// panic on total failure as this shouldn't happen.
-	log.Panicf(nil, "failed %s after %d tries", msg, count)
-}

internal/config/util.go

@@ -1,39 +0,0 @@
-// GoToSocial
-// Copyright (C) GoToSocial Authors admin@gotosocial.org
-// SPDX-License-Identifier: AGPL-3.0-or-later
-//
-// This program is free software: you can redistribute it and/or modify
-// it under the terms of the GNU Affero General Public License as published by
-// the Free Software Foundation, either version 3 of the License, or
-// (at your option) any later version.
-//
-// This program is distributed in the hope that it will be useful,
-// but WITHOUT ANY WARRANTY; without even the implied warranty of
-// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-// GNU Affero General Public License for more details.
-//
-// You should have received a copy of the GNU Affero General Public License
-// along with this program.  If not, see <http://www.gnu.org/licenses/>.
-
-package config
-
-import (
-	"net/netip"
-
-	"github.com/superseriousbusiness/gotosocial/internal/log"
-)
-
-func MustParseIPPrefixes(in []string) []netip.Prefix {
-	prefs := make([]netip.Prefix, 0, len(in))
-
-	for _, i := range in {
-		pref, err := netip.ParsePrefix(i)
-		if err != nil {
-			log.Panicf(nil, "error parsing ip prefix from %q: %v", i, err)
-		}
-
-		prefs = append(prefs, pref)
-	}
-
-	return prefs
-}

internal/transport/delivery/worker_test.go

@@ -24,11 +24,11 @@
 	"math/rand"
 	"net"
 	"net/http"
+	"net/netip"
 	"strconv"
 	"strings"
 	"testing"
 
-	"github.com/superseriousbusiness/gotosocial/internal/config"
 	"github.com/superseriousbusiness/gotosocial/internal/httpclient"
 	"github.com/superseriousbusiness/gotosocial/internal/queue"
 	"github.com/superseriousbusiness/gotosocial/internal/transport/delivery"
@@ -44,11 +44,8 @@ func TestDeliveryWorkerPool(t *testing.T) {
 
 func testDeliveryWorkerPool(t *testing.T, sz int, input []*testrequest) {
 	wp := new(delivery.WorkerPool)
-	wp.Init(httpclient.New(httpclient.Config{
+	allowLocal := []netip.Prefix{netip.MustParsePrefix("127.0.0.0/8")}
-		AllowRanges: config.MustParseIPPrefixes([]string{
+	wp.Init(httpclient.New(httpclient.Config{AllowRanges: allowLocal}))
-			"127.0.0.0/8",
-		}),
-	}))
 	wp.Start(sz)
 	defer wp.Stop()
 	test(t, &wp.Queue, input)