review: address dunglas's review batch

- BackgroundScope -> Scope (and NextScope / WithRequestScope /
  WithWorkerScope), keeping the type opaque + reusable for other
  per-server isolation contexts (Mercure hubs, future Prometheus
  labels). No behaviour change.
- $_SERVER['FRANKENPHP_WORKER'] now carries the user-facing worker
  name instead of "1". The doc has always told callers to rely on
  presence, not value. Drops the parallel FRANKENPHP_WORKER_NAME
  injection.
- backgroundWorkerReady.abortErr keeps the full error (not just its
  message), so the wrap chain survives into ensure() callers.
- Cap-error wording: drop the Caddyfile-specific "max_threads on the
  catch-all" phrasing for "increase max threads or declare it as a
  named worker", to read cleanly when the user is on the JSON config.
- Constants merged into a single block; getLookup folds the worker /
  background-worker handler cases via a small anonymous interface
  (Go's type switch can't fan out type-asserted access otherwise).
- Stub doc nits: third-person godoc ("Declares" / "Returns").
- Test ergonomics: requireSentinelEventually wraps the duplicated
  Eventually glue; require.NoFileExists, require.ErrorContains,
  assert.WithinDuration replace ad-hoc combinations.
- Renamed background_worker_*_test.go to bgworker*_test.go to match
  the bgworker.go source file (no underscores in file-name prefixes).

Author: nicolas-grekas

background_worker_batch_test.go

@@ -13,14 +13,16 @@ import (
 	"unsafe"
 )
 
-// defaultMaxBackgroundWorkers is the default safety cap for catch-all
-// background workers when the user doesn't set max_threads. Caps the
-// number of distinct lazy-started instances from a single catch-all.
-const defaultMaxBackgroundWorkers = 16
-
-// defaultEnsureTimeout is the default deadline applied when ensure() is
-// called without an explicit timeout.
-const defaultEnsureTimeout = 30 * time.Second
+const (
+	// defaultMaxBackgroundWorkers is the default safety cap for catch-all
+	// background workers when the user doesn't set max_threads. Caps the
+	// number of distinct lazy-started instances from a single catch-all.
+	defaultMaxBackgroundWorkers = 16
+
+	// defaultEnsureTimeout is the default deadline applied when ensure() is
+	// called without an explicit timeout.
+	defaultEnsureTimeout = 30 * time.Second
+)
 
 // backgroundWorkerReady is the per-instance readiness signal ensure()
 // blocks on. ready closes once the worker calls
@@ -32,7 +34,7 @@ type backgroundWorkerReady struct {
 
 	aborted   chan struct{}
 	abortOnce sync.Once
-	abortErr  string
+	abortErr  error
 
 	// bootFailure carries the most recent pre-readiness crash so a timing-
 	// out ensure() can report it instead of a generic "didn't call
@@ -67,26 +69,26 @@ func (r *backgroundWorkerReady) markReady() {
 // (max_consecutive_failures hit before readiness). Idempotent.
 func (r *backgroundWorkerReady) abort(err error) {
 	r.abortOnce.Do(func() {
-		r.abortErr = err.Error()
+		r.abortErr = err
 		close(r.aborted)
 	})
 }
 
-// BackgroundScope is an opaque per-php_server isolation boundary; the
+// Scope is an opaque per-php_server isolation boundary; the
 // zero value is the global/embed scope. Obtain values via
-// NextBackgroundWorkerScope.
-type BackgroundScope uint64
+// NextScope.
+type Scope uint64
 
-var backgroundScopeCounter atomic.Uint64
+var scopeCounter atomic.Uint64
 
-// NextBackgroundWorkerScope returns a fresh scope value. Each php_server
+// NextScope returns a fresh scope value. Each php_server
 // block should call this once during provisioning.
-func NextBackgroundWorkerScope() BackgroundScope {
-	return BackgroundScope(backgroundScopeCounter.Add(1))
+func NextScope() Scope {
+	return Scope(scopeCounter.Add(1))
 }
 
 // backgroundLookups maps scope -> lookup. Scope 0 is the global/embed scope.
-var backgroundLookups map[BackgroundScope]*backgroundWorkerLookup
+var backgroundLookups map[Scope]*backgroundWorkerLookup
 
 // backgroundWorkerLookup resolves a user-facing worker name to its *worker;
 // catchAll is the fallback when byName misses.
@@ -113,8 +115,8 @@ func (l *backgroundWorkerLookup) resolve(name string) *worker {
 // lookup. Per-scope name collisions are caught here because bg workers
 // intentionally skip the global workersByName map (so two scopes can share
 // a user-facing name).
-func buildBackgroundWorkerLookups(workers []*worker, opts []workerOpt) (map[BackgroundScope]*backgroundWorkerLookup, error) {
-	lookups := make(map[BackgroundScope]*backgroundWorkerLookup)
+func buildBackgroundWorkerLookups(workers []*worker, opts []workerOpt) (map[Scope]*backgroundWorkerLookup, error) {
+	lookups := make(map[Scope]*backgroundWorkerLookup)
 
 	for i, o := range opts {
 		if !o.isBackgroundWorker {
@@ -171,17 +173,15 @@ func getLookup(thread *phpThread) *backgroundWorkerLookup {
 	if backgroundLookups == nil {
 		return nil
 	}
-	var scope BackgroundScope
+	var scope Scope
 	if thread != nil {
-		switch handler := thread.handler.(type) {
-		case *workerThread:
-			scope = handler.worker.backgroundScope
-		case *backgroundWorkerThread:
-			scope = handler.worker.backgroundScope
-		default:
-			if fc, ok := fromContext(thread.context()); ok {
-				scope = fc.backgroundScope
-			}
+		// Both *workerThread and *backgroundWorkerThread expose their *worker
+		// via this anonymous interface; classic-mode threads don't and fall
+		// through to the request-context path.
+		if h, ok := thread.handler.(interface{ scopedWorker() *worker }); ok {
+			scope = h.scopedWorker().backgroundScope
+		} else if fc, ok := fromContext(thread.context()); ok {
+			scope = fc.backgroundScope
 		}
 	}
 	if scope != 0 {
@@ -228,13 +228,13 @@ func ensureBackgroundWorker(thread *phpThread, bgWorkerName string, timeout time
 
 	if catchAll.catchAllCap > 0 && len(catchAll.catchAllNames) >= catchAll.catchAllCap {
 		catchAll.catchAllMu.Unlock()
-		return fmt.Errorf("cannot start background worker %q: limit of %d reached (increase max_threads on the catch-all background worker or declare it as a named worker)", bgWorkerName, catchAll.catchAllCap)
+		return fmt.Errorf("cannot start background worker %q: limit of %d reached (increase max threads or declare it as a named worker)", bgWorkerName, catchAll.catchAllCap)
 	}
 
 	t := getInactivePHPThread()
 	if t == nil {
 		catchAll.catchAllMu.Unlock()
-		return fmt.Errorf("no available PHP thread for background worker (increase max_threads)")
+		return fmt.Errorf("no available PHP thread for background worker (increase max threads)")
 	}
 
 	r := newBackgroundWorkerReady()
@@ -264,7 +264,7 @@ func lazyStartNamedWorker(w *worker) error {
 	}
 	t := getInactivePHPThread()
 	if t == nil {
-		return fmt.Errorf("no available PHP thread for background worker (increase max_threads)")
+		return fmt.Errorf("no available PHP thread for background worker (increase max threads)")
 	}
 	convertToBackgroundWorkerThread(t, w, w.name, w.backgroundReady)
 	w.bgLazyStarted = true
@@ -287,7 +287,7 @@ func waitForBackgroundWorkerReady(name string, r *backgroundWorkerReady, timeout
 	case <-r.ready:
 		return nil
 	case <-r.aborted:
-		return fmt.Errorf("background worker %q failed to start: %s", name, r.abortErr)
+		return fmt.Errorf("background worker %q failed to start: %w", name, r.abortErr)
 	case <-timer.C:
 		return formatEnsureTimeoutError(name, r, timeout)
 	}