lolly/docs/superpowers/plans/2026-06-11-bug-fixes.md

16 KiB
Raw Blame History

Lolly 实际问题修复实施计划

For agentic workers: REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (- [ ]) syntax for tracking.

Goal: 修复 code review 中识别的关键实际问题和风险:配置默认值未应用、状态端点注册条件错误、生产代码中的非 zerolog 日志、以及小技术债。

Architecture: Task A 和 Task B 有依赖关系Task A 修复默认值合并后,会让 Monitoring.Status.PathAllow 变成非空,从而触发 startSingleMode 中错误的注册条件(原本用 Path/Allow 判断,应该改为 Enabled。因此两个修复必须在一个 commit 中或按正确顺序提交。Task C 独立。

Tech Stack: Go 1.26+, zerolog, fasthttp, yaml.v3


文件结构映射

Task A+B: 配置默认值合并 + Monitoring 注册条件修复 + 日志统一

  • Modify: internal/config/config.goLoad() 从 DefaultConfig() 开始 unmarshal
  • Create: internal/config/load_test.go — 默认值合并测试
  • Modify: internal/server/server.go — monitoring 注册条件改为 Enabled 检查
  • Create: internal/server/monitoring_registration_test.go — 注册条件回归测试
  • Modify: internal/server/status.go — 替换 4 处 log.Printf 为 zerolog
  • Modify: internal/lua/api_timer.go — 替换 4 处 log.Printf 为 zerolog

Task C: 小技术债清理

  • Modify: internal/proxy/proxy.go — 删除 //go:generate go test -v ./...

Task A+B: 配置默认值合并 + Monitoring 条件修复 + 日志统一

子任务 A1: 实现 config.Load() 默认值合并

Files:

  • Modify: internal/config/config.go:131-158

  • Create: internal/config/load_test.go

  • Step A1.1: 编写失败测试验证当前行为

创建 internal/config/load_test.go

package config

import (
	"os"
	"path/filepath"
	"testing"
	"time"
)

func TestLoad_MergesDefaults(t *testing.T) {
	tmpDir := t.TempDir()
	path := filepath.Join(tmpDir, "minimal.yaml")
	content := `
servers:
  - listen: ":8080"
`
	if err := os.WriteFile(path, []byte(content), 0o644); err != nil {
		t.Fatalf("write config: %v", err)
	}

	cfg, err := Load(path)
	if err != nil {
		t.Fatalf("Load failed: %v", err)
	}

	if cfg.Performance.FileCache.MaxEntries == 0 {
		t.Error("Performance.FileCache.MaxEntries should have default value")
	}
	if cfg.Performance.FileCache.MaxSize == 0 {
		t.Error("Performance.FileCache.MaxSize should have default value")
	}
	if cfg.Performance.FileCache.Inactive == 0 {
		t.Error("Performance.FileCache.Inactive should have default value")
	}
	if cfg.Monitoring.Status.Path != "/_status" {
		t.Errorf("Monitoring.Status.Path = %q, want %q", cfg.Monitoring.Status.Path, "/_status")
	}
	if cfg.Monitoring.Pprof.Path != "/debug/pprof" {
		t.Errorf("Monitoring.Pprof.Path = %q, want %q", cfg.Monitoring.Pprof.Path, "/debug/pprof")
	}
	if cfg.Resolver.CacheTTL == 0 {
		t.Error("Resolver.CacheTTL should have default value")
	}
}

func TestLoad_ExplicitOverridesDefault(t *testing.T) {
	tmpDir := t.TempDir()
	path := filepath.Join(tmpDir, "explicit.yaml")
	content := `
performance:
  file_cache:
    max_entries: 500
    max_size: 52428800
    inactive: 120s
servers:
  - listen: ":8080"
`
	if err := os.WriteFile(path, []byte(content), 0o644); err != nil {
		t.Fatalf("write config: %v", err)
	}

	cfg, err := Load(path)
	if err != nil {
		t.Fatalf("Load failed: %v", err)
	}

	if cfg.Performance.FileCache.MaxEntries != 500 {
		t.Errorf("MaxEntries = %d, want 500", cfg.Performance.FileCache.MaxEntries)
	}
	if cfg.Performance.FileCache.MaxSize != 52428800 {
		t.Errorf("MaxSize = %d, want 52428800", cfg.Performance.FileCache.MaxSize)
	}
	if cfg.Performance.FileCache.Inactive != 120*time.Second {
		t.Errorf("Inactive = %v, want 120s", cfg.Performance.FileCache.Inactive)
	}
}

func TestLoad_MonitoringDisabledByDefault(t *testing.T) {
	tmpDir := t.TempDir()
	path := filepath.Join(tmpDir, "minimal.yaml")
	content := `
servers:
  - listen: ":8080"
`
	if err := os.WriteFile(path, []byte(content), 0o644); err != nil {
		t.Fatalf("write config: %v", err)
	}

	cfg, err := Load(path)
	if err != nil {
		t.Fatalf("Load failed: %v", err)
	}

	// 默认值 Path 存在,但 Enabled 应为 false
	if cfg.Monitoring.Status.Enabled {
		t.Error("Monitoring.Status.Enabled should be false by default")
	}
	if cfg.Monitoring.Pprof.Enabled {
		t.Error("Monitoring.Pprof.Enabled should be false by default")
	}
	if cfg.Monitoring.Status.Path != "/_status" {
		t.Errorf("Monitoring.Status.Path = %q, want %q", cfg.Monitoring.Status.Path, "/_status")
	}
}

运行:

go test -v -run "TestLoad_" ./internal/config/...

Expected: FAIL — TestLoad_MergesDefaultsTestLoad_MonitoringDisabledByDefault 失败

  • Step A1.2: 修改 Load() 从 DefaultConfig() 开始

修改 internal/config/config.go

func Load(path string) (*Config, error) {
	data, err := os.ReadFile(path)
	if err != nil {
		return nil, fmt.Errorf("读取配置文件失败: %w", err)
	}

	// 从默认值开始YAML 只覆盖显式配置的字段。
	// 注意yaml.v3 对 slice 会整体替换,因此用户显式配置的 Servers[]
	// 元素不会继承 server-level 默认值;但顶层 struct 字段Performance、
	// Monitoring、Resolver的默认值会被保留。
	cfg := DefaultConfig()
	if err := yaml.Unmarshal(data, cfg); err != nil {
		return nil, fmt.Errorf("解析配置文件失败: %w", err)
	}

	if len(cfg.Include) > 0 {
		absPath, err := filepath.Abs(path)
		if err != nil {
			return nil, fmt.Errorf("获取配置文件绝对路径失败: %w", err)
		}
		visited := map[string]bool{absPath: true}
		if err := processIncludes(cfg, filepath.Dir(path), 0, visited); err != nil {
			return nil, fmt.Errorf("处理配置引入失败: %w", err)
		}
	}

	if err := Validate(cfg); err != nil {
		return nil, fmt.Errorf("配置验证失败: %w", err)
	}

	return cfg, nil
}
  • Step A1.3: 运行测试确认 A1 修复
go test -v -run "TestLoad_" ./internal/config/...

Expected: TestLoad_MergesDefaults PASS, TestLoad_ExplicitOverridesDefault PASS, TestLoad_MonitoringDisabledByDefault PASS


子任务 B1: 修复 monitoring 注册条件(防止 Task A 引入 regression

Files:

  • Modify: internal/server/server.go:478-490, 492-508, 510-520

背景: startSingleMode 当前用 Path != "" || len(Allow) > 0 判断是否注册 status handler。Task A 修复后,Monitoring.Status.Path 默认值变成 "/_status"Allow 默认也有值,这会导致即使用户没有启用 monitoringhandler 也会被注册。必须改为检查 Enabled,与 vhost 模式保持一致。

  • Step B1.1: 修改 status handler 注册条件

internal/server/server.go 第 478-490 行:

// BEFORE
if s.config.Monitoring.Status.Path != "" || len(s.config.Monitoring.Status.Allow) > 0 {
    statusHandler, err := NewStatusHandler(s, &s.config.Monitoring.Status)
    ...
}

// AFTER
if s.config.Monitoring.Status.Enabled {
    statusHandler, err := NewStatusHandler(s, &s.config.Monitoring.Status)
    ...
}
  • Step B1.2: 确认 pprof 和 purge 使用 Enabled 检查

pprof 已经是 if s.config.Monitoring.Pprof.Enabled {,保持不变。 purge 已经是 if serverCfg.CacheAPI != nil && serverCfg.CacheAPI.Enabled {,保持不变。

  • Step B1.3: 添加注册条件回归测试

创建 internal/server/monitoring_registration_test.go

package server

import (
	"testing"

	"github.com/stretchr/testify/assert"
	"github.com/valyala/fasthttp"
	"rua.plus/lolly/internal/config"
)

func TestMonitoringEndpoints_OnlyRegisteredWhenEnabled(t *testing.T) {
	// Case 1: monitoring 未启用时,/_status 不应注册
	cfg := &config.Config{
		Servers: []config.ServerConfig{{
			Listen: "127.0.0.1:0",
			Static: []config.StaticConfig{{
				Path: "/",
				Root: t.TempDir(),
			}},
		}},
	}

	srv := New(cfg)
	go srv.Start()
	defer srv.Stop()

	listeners := srv.GetListeners()
	if len(listeners) == 0 {
		t.Fatal("server has no listeners")
	}
	addr := listeners[0].Addr().String()

	client := &fasthttp.Client{}
	req := fasthttp.AcquireRequest()
	resp := fasthttp.AcquireResponse()
	defer fasthttp.ReleaseRequest(req)
	defer fasthttp.ReleaseResponse(resp)

	req.SetRequestURI("http://" + addr + "/_status")
	req.Header.SetMethod("GET")

	if err := client.Do(req, resp); err != nil {
		t.Fatalf("request failed: %v", err)
	}

	// 未启用时应返回 404被 static handler 处理,找不到文件)
	assert.Equal(t, fasthttp.StatusNotFound, resp.StatusCode(),
		"status endpoint should NOT be registered when monitoring is disabled")
}

func TestMonitoringEndpoints_ReachableWhenEnabled(t *testing.T) {
	cfg := &config.Config{
		Monitoring: config.MonitoringConfig{
			Status: config.StatusConfig{
				Enabled: true,
				Path:    "/_status",
				Allow:   []string{"127.0.0.1"},
			},
		},
		Servers: []config.ServerConfig{{
			Listen: "127.0.0.1:0",
			Static: []config.StaticConfig{{
				Path: "/",
				Root: t.TempDir(),
			}},
		}},
	}

	srv := New(cfg)
	go srv.Start()
	defer srv.Stop()

	listeners := srv.GetListeners()
	if len(listeners) == 0 {
		t.Fatal("server has no listeners")
	}
	addr := listeners[0].Addr().String()

	client := &fasthttp.Client{}
	req := fasthttp.AcquireRequest()
	resp := fasthttp.AcquireResponse()
	defer fasthttp.ReleaseRequest(req)
	defer fasthttp.ReleaseResponse(resp)

	req.SetRequestURI("http://" + addr + "/_status")
	req.Header.SetMethod("GET")

	if err := client.Do(req, resp); err != nil {
		t.Fatalf("request failed: %v", err)
	}

	assert.Equal(t, fasthttp.StatusOK, resp.StatusCode(),
		"status endpoint should be reachable when enabled, even with static handler on /")
}

func TestLocationEngine_StatusExactBeatsStaticPrefix(t *testing.T) {
	// 独立验证 location engine 的优先级exact match 应该 beat prefix /
	engine := matcher.NewLocationEngine()

	var exactCalled, prefixCalled bool
	engine.AddExact("/_status", func(ctx *fasthttp.RequestCtx) {
		exactCalled = true
		ctx.SetStatusCode(fasthttp.StatusOK)
	}, false)
	engine.AddPrefix("/", func(ctx *fasthttp.RequestCtx) {
		prefixCalled = true
		ctx.SetStatusCode(fasthttp.StatusNotFound)
	}, false)
	engine.MarkInitialized()

	result := engine.Match([]byte("/_status"))
	if result == nil {
		t.Fatal("expected match")
	}
	if result.LocationType != matcher.LocationTypeExact {
		t.Errorf("expected exact match, got %s", result.LocationType)
	}
}

注意:需要导入 rua.plus/lolly/internal/matcher

  • Step B1.4: 运行测试
go test -v -run "TestMonitoringEndpoints|TestLocationEngine" ./internal/server/...

Expected:

  • TestMonitoringEndpoints_OnlyRegisteredWhenEnabled PASS返回 404
  • TestMonitoringEndpoints_ReachableWhenEnabled PASS返回 200
  • TestLocationEngine_StatusExactBeatsStaticPrefix PASS

子任务 B2: 替换生产代码中的 log.Printf

Files:

  • Modify: internal/server/status.go

  • Modify: internal/lua/api_timer.go

  • Step B2.1: 替换 status.go 中的 log.Printf

修改 internal/server/status.go

  • 删除 "log" import
  • 添加 "rua.plus/lolly/internal/logging" import如果不存在

替换 4 处:

// 约 302 行
logging.Error().Err(err).Msg("failed to write metrics response")

// 约 318 行
logging.Error().Err(err).Msg("failed to write status response")

// 约 375 行
logging.Error().Err(err).Msg("failed to write text response")

// 约 480 行
logging.Error().Err(err).Msg("failed to write html response")
  • Step B2.2: 替换 lua/api_timer.go 中的 log.Printf

修改 internal/lua/api_timer.go

  • 删除 "log" import
  • 添加 "rua.plus/lolly/internal/logging" import

替换 4 处:

// 约 264 行
logging.Warn().Msg("[lua] timer callback dropped: queue full")

// 约 280 行
logging.Error().Msg("[lua] timer callback: failed to create function from proto")

// 约 289 行
logging.Error().Err(err).Msg("[lua] timer callback error")

// 约 408 行
logging.Warn().Int("abandoned", abandoned).Msg("[lua] shutdown timeout: callbacks abandoned")
  • Step B2.3: 运行测试
go test -v ./internal/server/... ./internal/lua/...

Expected: PASS


子任务 A+B 提交

Task A 和 Task B1 必须一起提交,否则单独提交 A 会引入 regression。

git add internal/config/config.go internal/config/load_test.go
-git add internal/server/server.go internal/server/monitoring_registration_test.go
-git add internal/server/status.go internal/lua/api_timer.go
git commit -m "fix(config,server): merge defaults on Load and fix monitoring registration

Two related fixes that must land together:

1. config.Load() now starts from DefaultConfig() before unmarshaling
   YAML. This ensures missing top-level fields (Performance,
   Monitoring, Resolver) use their documented defaults instead of
   zero values. Most importantly, file_cache is no longer silently
   disabled when users omit the performance: section.

2. startSingleMode() now checks Monitoring.Status.Enabled instead of
   Path/Allow to decide whether to register the status endpoint.
   Without this change, fix #1 would have caused a regression where
   the status handler is registered even when monitoring is disabled,
   because DefaultConfig() sets Path and Allow defaults.

Also replace remaining log.Printf in status.go and lua/api_timer.go
with zerolog to follow project logging conventions.

Added tests:
- config/load_test.go: verifies defaults are applied, explicit values
  override defaults, and monitoring stays disabled by default.
- server/monitoring_registration_test.go: verifies /_status is only
  registered when enabled and remains reachable with static handler
  on path: /."

Task C: 清理 proxy.go 的 go:generate

Files:

  • Modify: internal/proxy/proxy.go:31

  • Step C1: 删除不合理的 go:generate

删除 internal/proxy/proxy.go 第 31 行:

//go:generate go test -v ./...
  • Step C2: 运行测试
go test ./internal/proxy/...

Expected: PASS

  • Step C3: Commit Task C
git add internal/proxy/proxy.go
git commit -m "chore(proxy): remove misplaced go:generate directive

The //go:generate go test -v ./... directive is not what
go:generate is intended for (code generation, not test execution).
Remove it to avoid confusion."

Task D: 最终验证

  • Step D1: 运行全量测试
go test ./internal/... -count=1 2>&1 | tail -30

Expected: 除已知 integration DNS 测试外全部 PASS

  • Step D2: 运行 race detector
go test -race ./internal/config/... ./internal/server/... ./internal/lua/... ./internal/proxy/... -count=1

Expected: 零 race

  • Step D3: 运行 linter
make lint

Expected: 0 issues

  • Step D4: 检查无 log.Printf 残留
grep -rn "^[[:space:]]*log\.Printf\|^[[:space:]]*log\.Println\|^[[:space:]]*log\.Fatal" internal/ --include='*.go' | grep -v '_test.go' | grep -v '//'

Expected: 空(app/import.go 中的 fmt.Printf 是 CLI 输出,允许保留)

  • Step D5: 运行 build
make build

Expected: 构建成功


Spec Coverage Check

发现的问题 对应修复
config.Load() 不合并 DefaultConfig() Task A: Load() 从 DefaultConfig() 开始 unmarshal
Task A 会引入 monitoring Enabled 回归 Task B1: 注册条件改为 Enabled
status 端点未启用时返回 404 / 启用时不工作 Task B1: Enabled 条件 + 回归测试
server/status.go 使用 log.Printf Task B2: 替换为 zerolog
lua/api_timer.go 使用 log.Printf Task B2: 替换为 zerolog
proxy.go 的 go:generate 不合理 Task C: 删除

附录:常用验证命令

# 关键测试
go test -v -run "TestLoad_" ./internal/config/...
go test -v -run "TestMonitoringEndpoints|TestLocationEngine" ./internal/server/...

# 相关包
go test ./internal/config/... ./internal/server/... ./internal/lua/... ./internal/proxy/...

# 全量
go test ./internal/... -count=1

# Lint + Build
make lint
make build