From 8092f4a35c1958b39a4990a85f438598657d7ab5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dunglas?= Date: Thu, 17 Apr 2025 20:33:22 +0200 Subject: [PATCH] chore!: update to golangci-lint-action 7 (#1508) --- .github/workflows/tests.yaml | 2 +- caddy/admin.go | 2 +- cgi.go | 4 ++-- frankenphp.go | 26 +++++++++++++------------- frankenphp_test.go | 2 +- internal/watcher/watcher.go | 12 ++++++------ metrics.go | 11 ++++++----- phpmainthread.go | 11 +---------- phpmainthread_test.go | 2 +- phpthread.go | 2 +- scaling.go | 12 ++++-------- threadinactive.go | 2 +- threadregular.go | 2 +- worker_test.go | 4 +++- 14 files changed, 42 insertions(+), 52 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index f4ae6b88..d485fcce 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -68,7 +68,7 @@ jobs: - name: Run integrations tests run: ./reload_test.sh - name: Lint Go code - uses: golangci/golangci-lint-action@v6 + uses: golangci/golangci-lint-action@v7 if: matrix.php-versions == '8.4' with: version: latest diff --git a/caddy/admin.go b/caddy/admin.go index 2be765b0..ec6d7d7c 100644 --- a/caddy/admin.go +++ b/caddy/admin.go @@ -44,7 +44,7 @@ func (admin *FrankenPHPAdmin) restartWorkers(w http.ResponseWriter, r *http.Requ return nil } -func (admin *FrankenPHPAdmin) threads(w http.ResponseWriter, r *http.Request) error { +func (admin *FrankenPHPAdmin) threads(w http.ResponseWriter, _ *http.Request) error { debugState := frankenphp.DebugState() prettyJson, err := json.MarshalIndent(debugState, "", " ") if err != nil { diff --git a/cgi.go b/cgi.go index 9088e83a..6f31921b 100644 --- a/cgi.go +++ b/cgi.go @@ -168,7 +168,7 @@ func packCgiVariable(key *C.zend_string, value string) C.ht_key_value_pair { return C.ht_key_value_pair{key, toUnsafeChar(value), C.size_t(len(value))} } -func addHeadersToServer(thread *phpThread, fc *frankenPHPContext, trackVarsArray *C.zval) { +func addHeadersToServer(fc *frankenPHPContext, trackVarsArray *C.zval) { for field, val := range fc.request.Header { if k := mainThread.commonHeaders[field]; k != nil { v := strings.Join(val, ", ") @@ -197,7 +197,7 @@ func go_register_variables(threadIndex C.uintptr_t, trackVarsArray *C.zval) { fc := thread.getRequestContext() addKnownVariablesToServer(thread, fc, trackVarsArray) - addHeadersToServer(thread, fc, trackVarsArray) + addHeadersToServer(fc, trackVarsArray) // The Prepared Environment is registered last and can overwrite any previous values addPreparedEnvToServer(fc, trackVarsArray) diff --git a/frankenphp.go b/frankenphp.go index 17caea1c..24469356 100644 --- a/frankenphp.go +++ b/frankenphp.go @@ -55,13 +55,13 @@ type contextKeyStruct struct{} var contextKey = contextKeyStruct{} var ( - InvalidRequestError = errors.New("not a FrankenPHP request") - AlreadyStartedError = errors.New("FrankenPHP is already started") - InvalidPHPVersionError = errors.New("FrankenPHP is only compatible with PHP 8.2+") - MainThreadCreationError = errors.New("error creating the main thread") - RequestContextCreationError = errors.New("error during request context creation") - ScriptExecutionError = errors.New("error during PHP script execution") - NotRunningError = errors.New("FrankenPHP is not running. For proper configuration visit: https://frankenphp.dev/docs/config/#caddyfile-config") + ErrInvalidRequest = errors.New("not a FrankenPHP request") + ErrAlreadyStarted = errors.New("FrankenPHP is already started") + ErrInvalidPHPVersion = errors.New("FrankenPHP is only compatible with PHP 8.2+") + ErrMainThreadCreation = errors.New("error creating the main thread") + ErrRequestContextCreation = errors.New("error during request context creation") + ErrScriptExecution = errors.New("error during PHP script execution") + ErrNotRunning = errors.New("FrankenPHP is not running. For proper configuration visit: https://frankenphp.dev/docs/config/#caddyfile-config") isRunning bool @@ -189,7 +189,7 @@ func calculateMaxThreads(opt *opt) (int, int, int, error) { return opt.numThreads, numWorkers, opt.maxThreads, nil } - if !numThreadsIsSet && !maxThreadsIsSet { + if !numThreadsIsSet { if numWorkers >= maxProcs { // Start at least as many threads as workers, and keep a free thread to handle requests in non-worker mode opt.numThreads = numWorkers + 1 @@ -218,7 +218,7 @@ func calculateMaxThreads(opt *opt) (int, int, int, error) { // Init starts the PHP runtime and the configured workers. func Init(options ...Option) error { if isRunning { - return AlreadyStartedError + return ErrAlreadyStarted } isRunning = true @@ -265,7 +265,7 @@ func Init(options ...Option) error { config := Config() if config.Version.MajorVersion < 8 || (config.Version.MajorVersion == 8 && config.Version.MinorVersion < 2) { - return InvalidPHPVersionError + return ErrInvalidPHPVersion } if config.ZTS { @@ -381,7 +381,7 @@ func updateServerContext(thread *phpThread, fc *frankenPHPContext, isWorkerReque ) if ret > 0 { - return RequestContextCreationError + return ErrRequestContextCreation } return nil @@ -390,12 +390,12 @@ func updateServerContext(thread *phpThread, fc *frankenPHPContext, isWorkerReque // ServeHTTP executes a PHP script according to the given context. func ServeHTTP(responseWriter http.ResponseWriter, request *http.Request) error { if !isRunning { - return NotRunningError + return ErrNotRunning } fc, ok := fromContext(request.Context()) if !ok { - return InvalidRequestError + return ErrInvalidRequest } fc.responseWriter = responseWriter diff --git a/frankenphp_test.go b/frankenphp_test.go index 59312c90..efd226d8 100644 --- a/frankenphp_test.go +++ b/frankenphp_test.go @@ -754,7 +754,7 @@ func testFileUpload(t *testing.T, opts *testOptions) { _, err := part.Write([]byte("bar")) require.NoError(t, err) - writer.Close() + require.NoError(t, writer.Close()) req := httptest.NewRequest("POST", "http://example.com/file-upload.php", requestBody) req.Header.Add("Content-Type", writer.FormDataContentType()) diff --git a/internal/watcher/watcher.go b/internal/watcher/watcher.go index abca9d82..c420f32f 100644 --- a/internal/watcher/watcher.go +++ b/internal/watcher/watcher.go @@ -37,14 +37,15 @@ var failureMu = sync.Mutex{} var watcherIsActive = atomic.Bool{} var ( + ErrAlreadyStarted = errors.New("the watcher is already running") + ErrUnableToStartWatching = errors.New("unable to start the watcher") + // the currently active file watcher activeWatcher *watcher // after stopping the watcher we will wait for eventual reloads to finish reloadWaitGroup sync.WaitGroup // we are passing the logger from the main package to the watcher - logger *zap.Logger - AlreadyStartedError = errors.New("the watcher is already running") - UnableToStartWatching = errors.New("unable to start the watcher") + logger *zap.Logger ) func InitWatcher(filePatterns []string, callback func(), zapLogger *zap.Logger) error { @@ -52,7 +53,7 @@ func InitWatcher(filePatterns []string, callback func(), zapLogger *zap.Logger) return nil } if watcherIsActive.Load() { - return AlreadyStartedError + return ErrAlreadyStarted } watcherIsActive.Store(true) logger = zapLogger @@ -142,7 +143,7 @@ func startSession(w *watchPattern) (C.uintptr_t, error) { } logger.Error("couldn't start watching", zap.String("dir", w.dir)) - return watchSession, UnableToStartWatching + return watchSession, ErrUnableToStartWatching } func stopSession(session C.uintptr_t) { @@ -186,7 +187,6 @@ func listenForFileEvents(triggerWatcher chan string, stopWatcher chan struct{}) for { select { case <-stopWatcher: - break case lastChangedFile = <-triggerWatcher: timer.Reset(debounceDuration) case <-timer.C: diff --git a/metrics.go b/metrics.go index b5f4f69b..4de6d5e4 100644 --- a/metrics.go +++ b/metrics.go @@ -11,7 +11,7 @@ import ( const ( StopReasonCrash = iota StopReasonRestart - StopReasonShutdown + //StopReasonShutdown ) type StopReason int @@ -74,9 +74,9 @@ func (n nullMetrics) StartWorkerRequest(string) { func (n nullMetrics) Shutdown() { } -func (n nullMetrics) QueuedWorkerRequest(name string) {} +func (n nullMetrics) QueuedWorkerRequest(string) {} -func (n nullMetrics) DequeuedWorkerRequest(name string) {} +func (n nullMetrics) DequeuedWorkerRequest(string) {} func (n nullMetrics) QueuedRequest() {} func (n nullMetrics) DequeuedRequest() {} @@ -127,9 +127,10 @@ func (m *PrometheusMetrics) StopWorker(name string, reason StopReason) { m.totalWorkers.WithLabelValues(name).Dec() m.readyWorkers.WithLabelValues(name).Dec() - if reason == StopReasonCrash { + switch reason { + case StopReasonCrash: m.workerCrashes.WithLabelValues(name).Inc() - } else if reason == StopReasonRestart { + case StopReasonRestart: m.workerRestarts.WithLabelValues(name).Inc() } } diff --git a/phpmainthread.go b/phpmainthread.go index 7245db37..f3e18363 100644 --- a/phpmainthread.go +++ b/phpmainthread.go @@ -107,7 +107,7 @@ func drainPHPThreads() { func (mainThread *phpMainThread) start() error { if C.frankenphp_new_main_thread(C.int(mainThread.numThreads)) != 0 { - return MainThreadCreationError + return ErrMainThreadCreation } mainThread.state.waitFor(stateReady) @@ -144,15 +144,6 @@ func getInactivePHPThread() *phpThread { return nil } -func getPHPThreadAtState(state stateID) *phpThread { - for _, thread := range phpThreads { - if thread.state.is(state) { - return thread - } - } - return nil -} - //export go_frankenphp_main_thread_is_ready func go_frankenphp_main_thread_is_ready() { mainThread.setAutomaticMaxThreads() diff --git a/phpmainthread_test.go b/phpmainthread_test.go index f75e72b1..7639c825 100644 --- a/phpmainthread_test.go +++ b/phpmainthread_test.go @@ -220,7 +220,7 @@ func allPossibleTransitions(worker1Path string, worker2Path string) []func(*phpT func TestCorrectThreadCalculation(t *testing.T) { maxProcs := runtime.GOMAXPROCS(0) * 2 - oneWorkerThread := []workerOpt{workerOpt{num: 1}} + oneWorkerThread := []workerOpt{{num: 1}} // default values testThreadCalculation(t, maxProcs, maxProcs, &opt{}) diff --git a/phpthread.go b/phpthread.go index 737a79bc..933acbd6 100644 --- a/phpthread.go +++ b/phpthread.go @@ -149,7 +149,7 @@ func go_frankenphp_before_script_execution(threadIndex C.uintptr_t) *C.char { func go_frankenphp_after_script_execution(threadIndex C.uintptr_t, exitStatus C.int) { thread := phpThreads[threadIndex] if exitStatus < 0 { - panic(ScriptExecutionError) + panic(ErrScriptExecution) } thread.handler.afterScriptExecution(int(exitStatus)) diff --git a/scaling.go b/scaling.go index 0350e750..a5ceb673 100644 --- a/scaling.go +++ b/scaling.go @@ -20,8 +20,6 @@ const ( cpuProbeTime = 120 * time.Millisecond // do not scale over this amount of CPU usage maxCpuUsageForScaling = 0.8 - // upscale stalled threads every x milliseconds - upscaleCheckTime = 100 * time.Millisecond // downscale idle threads every x seconds downScaleCheckTime = 5 * time.Second // max amount of threads stopped in one iteration of downScaleCheckTime @@ -31,13 +29,11 @@ const ( ) var ( + ErrMaxThreadsReached = errors.New("max amount of overall threads reached") + scaleChan chan *frankenPHPContext autoScaledThreads = []*phpThread{} scalingMu = new(sync.RWMutex) - - MaxThreadsReachedError = errors.New("max amount of overall threads reached") - CannotRemoveLastThreadError = errors.New("cannot remove last thread") - WorkerNotFoundError = errors.New("worker not found for given filename") ) func initAutoScaling(mainThread *phpMainThread) { @@ -67,7 +63,7 @@ func drainAutoScaling() { func addRegularThread() (*phpThread, error) { thread := getInactivePHPThread() if thread == nil { - return nil, MaxThreadsReachedError + return nil, ErrMaxThreadsReached } convertToRegularThread(thread) thread.state.waitFor(stateReady, stateShuttingDown, stateReserved) @@ -77,7 +73,7 @@ func addRegularThread() (*phpThread, error) { func addWorkerThread(worker *worker) (*phpThread, error) { thread := getInactivePHPThread() if thread == nil { - return nil, MaxThreadsReachedError + return nil, ErrMaxThreadsReached } convertToWorkerThread(thread, worker) thread.state.waitFor(stateReady, stateShuttingDown, stateReserved) diff --git a/threadinactive.go b/threadinactive.go index bbdc167c..912d339f 100644 --- a/threadinactive.go +++ b/threadinactive.go @@ -33,7 +33,7 @@ func (handler *inactiveThread) beforeScriptExecution() string { panic("unexpected state: " + thread.state.name()) } -func (handler *inactiveThread) afterScriptExecution(exitStatus int) { +func (handler *inactiveThread) afterScriptExecution(int) { panic("inactive threads should not execute scripts") } diff --git a/threadregular.go b/threadregular.go index ac119208..5dee4550 100644 --- a/threadregular.go +++ b/threadregular.go @@ -47,7 +47,7 @@ func (handler *regularThread) beforeScriptExecution() string { } // return true if the worker should continue to run -func (handler *regularThread) afterScriptExecution(exitStatus int) { +func (handler *regularThread) afterScriptExecution(int) { handler.afterRequest() } diff --git a/worker_test.go b/worker_test.go index be404c2f..f3b9a9a7 100644 --- a/worker_test.go +++ b/worker_test.go @@ -2,6 +2,7 @@ package frankenphp_test import ( "fmt" + "github.com/stretchr/testify/require" "io" "log" "net/http" @@ -139,7 +140,8 @@ func ExampleServeHTTP_workers() { } func TestWorkerHasOSEnvironmentVariableInSERVER(t *testing.T) { - os.Setenv("CUSTOM_OS_ENV_VARIABLE", "custom_env_variable_value") + require.NoError(t, os.Setenv("CUSTOM_OS_ENV_VARIABLE", "custom_env_variable_value")) + runTest(t, func(handler func(http.ResponseWriter, *http.Request), _ *httptest.Server, i int) { req := httptest.NewRequest("GET", "http://example.com/worker.php", nil) w := httptest.NewRecorder()