diff --git a/error_test.go b/error_test.go index 0673626..68bdcc1 100644 --- a/error_test.go +++ b/error_test.go @@ -691,3 +691,58 @@ func TestError_MigrateErrorsExtended_Bad(t *testing.T) { assert.Error(t, err) }) } + +// TestError_EdgeCases_Ugly covers boundary conditions and degenerate inputs +// that fall outside normal error paths but are still reachable in production. +// +// store.saveState(nil) // nil state map is a no-op +// store.saveQuotas(nil) // nil quotas map is a no-op +func TestError_EdgeCases_Ugly(t *testing.T) { + t.Run("saveState with nil state map is a no-op", func(t *testing.T) { + databasePath := testPath(t.TempDir(), "nil-state.db") + store, err := newSQLiteStore(databasePath) + require.NoError(t, err) + defer store.close() + + err = store.saveState(nil) + assert.NoError(t, err, "nil state map should not cause an error") + }) + + t.Run("saveQuotas with nil quotas map is a no-op", func(t *testing.T) { + databasePath := testPath(t.TempDir(), "nil-quotas.db") + store, err := newSQLiteStore(databasePath) + require.NoError(t, err) + defer store.close() + + err = store.saveQuotas(nil) + assert.NoError(t, err, "nil quotas map should not cause an error") + }) + + t.Run("loadState on empty database returns empty map", func(t *testing.T) { + databasePath := testPath(t.TempDir(), "empty-load.db") + store, err := newSQLiteStore(databasePath) + require.NoError(t, err) + defer store.close() + + state, err := store.loadState() + require.NoError(t, err) + assert.Empty(t, state, "empty database should return empty state map") + }) + + t.Run("loadQuotas on empty database returns empty map", func(t *testing.T) { + databasePath := testPath(t.TempDir(), "empty-quotas.db") + store, err := newSQLiteStore(databasePath) + require.NoError(t, err) + defer store.close() + + quotas, err := store.loadQuotas() + require.NoError(t, err) + assert.Empty(t, quotas, "empty database should return empty quotas map") + }) + + t.Run("MigrateYAMLToSQLite with missing YAML file returns error", func(t *testing.T) { + tmpDir := t.TempDir() + err := MigrateYAMLToSQLite(testPath(tmpDir, "nonexistent.yaml"), testPath(tmpDir, "out.db")) + assert.Error(t, err, "migration from missing YAML should fail") + }) +} diff --git a/iter_test.go b/iter_test.go index d5f969b..239bf17 100644 --- a/iter_test.go +++ b/iter_test.go @@ -112,6 +112,45 @@ func TestIter_IterEarlyBreak_Good(t *testing.T) { }) } +// TestIter_CountTokensErrors_Bad covers error paths in CountTokens and +// countTokensWithClient that result from bad inputs or API failures. +// +// _, err := CountTokens(ctx, "key", "", "text") // returns error for empty model +func TestIter_CountTokensErrors_Bad(t *testing.T) { + t.Run("invalid base URL scheme returns error", func(t *testing.T) { + _, err := countTokensWithClient(context.Background(), nil, "://bad-url", "key", "model", "text") + assert.Error(t, err, "invalid base URL should return an error") + }) + + t.Run("empty base URL scheme returns error", func(t *testing.T) { + _, err := countTokensWithClient(context.Background(), nil, "not-a-url", "key", "model", "text") + assert.Error(t, err, "URL with no scheme/host should return an error") + }) + + t.Run("API returns invalid JSON body returns error", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write([]byte("this is not json")) + })) + defer server.Close() + + _, err := countTokensWithClient(context.Background(), server.Client(), server.URL, "key", "model", "text") + assert.Error(t, err, "invalid JSON response should return an error") + }) + + t.Run("API returns error body with unreadable response", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + w.Write([]byte("internal server error")) + })) + defer server.Close() + + _, err := countTokensWithClient(context.Background(), server.Client(), server.URL, "key", "model", "text") + require.Error(t, err) + assert.Contains(t, err.Error(), "status 500") + }) +} + func TestIter_CountTokensFull_Ugly(t *testing.T) { t.Run("empty model is rejected", func(t *testing.T) { _, err := CountTokens(context.Background(), "key", "", "text") diff --git a/ratelimit.go b/ratelimit.go index 9e7eee5..a5716e5 100644 --- a/ratelimit.go +++ b/ratelimit.go @@ -65,7 +65,7 @@ type ProviderProfile struct { // Config controls RateLimiter initialisation. // -// cfg := Config{Providers: []Provider{ProviderGemini}, FilePath: "/tmp/ratelimits.yaml"} +// configuration := Config{Providers: []Provider{ProviderGemini}, FilePath: "/tmp/ratelimits.yaml"} type Config struct { // FilePath overrides the default state file location. // If empty, defaults to ~/.core/ratelimits.yaml. @@ -179,13 +179,13 @@ func New() (*RateLimiter, error) { // If no providers or quotas are specified, Gemini defaults are used. // // rl, err := NewWithConfig(Config{Providers: []Provider{ProviderAnthropic}}) -func NewWithConfig(cfg Config) (*RateLimiter, error) { - backend, err := normaliseBackend(cfg.Backend) +func NewWithConfig(configuration Config) (*RateLimiter, error) { + backend, err := normaliseBackend(configuration.Backend) if err != nil { return nil, err } - filePath := cfg.FilePath + filePath := configuration.FilePath if filePath == "" { filePath, err = defaultStatePath(backend) if err != nil { @@ -194,15 +194,15 @@ func NewWithConfig(cfg Config) (*RateLimiter, error) { } if backend == backendSQLite { - if cfg.FilePath == "" { + if configuration.FilePath == "" { if err := ensureDir(core.PathDir(filePath)); err != nil { return nil, core.E("ratelimit.NewWithConfig", "mkdir", err) } } - return NewWithSQLiteConfig(filePath, cfg) + return NewWithSQLiteConfig(filePath, configuration) } - rl := newConfiguredRateLimiter(cfg) + rl := newConfiguredRateLimiter(configuration) rl.filePath = filePath return rl, nil } @@ -684,28 +684,28 @@ func (rl *RateLimiter) snapshotLocked(model string) ModelStats { } // NewWithSQLite creates a SQLite-backed RateLimiter with Gemini defaults. -// The database is created at dbPath if it does not exist. Use Close() to +// The database is created at databasePath if it does not exist. Use Close() to // release the database connection when finished. // // rl, err := NewWithSQLite("/tmp/ratelimits.db") -func NewWithSQLite(dbPath string) (*RateLimiter, error) { - return NewWithSQLiteConfig(dbPath, Config{ +func NewWithSQLite(databasePath string) (*RateLimiter, error) { + return NewWithSQLiteConfig(databasePath, Config{ Providers: []Provider{ProviderGemini}, }) } // NewWithSQLiteConfig creates a SQLite-backed RateLimiter with custom config. -// The Backend field in cfg is ignored (always "sqlite"). Use Close() to -// release the database connection when finished. +// The Backend field in configuration is ignored (always "sqlite"). Use Close() +// to release the database connection when finished. // // rl, err := NewWithSQLiteConfig("/tmp/ratelimits.db", Config{Providers: []Provider{ProviderOpenAI}}) -func NewWithSQLiteConfig(dbPath string, cfg Config) (*RateLimiter, error) { - store, err := newSQLiteStore(dbPath) +func NewWithSQLiteConfig(databasePath string, configuration Config) (*RateLimiter, error) { + store, err := newSQLiteStore(databasePath) if err != nil { return nil, err } - rl := newConfiguredRateLimiter(cfg) + rl := newConfiguredRateLimiter(configuration) rl.sqlite = store return rl, nil } @@ -821,20 +821,20 @@ func countTokensWithClient(ctx context.Context, client *http.Client, baseURL, ap return result.TotalTokens, nil } -func newConfiguredRateLimiter(cfg Config) *RateLimiter { +func newConfiguredRateLimiter(configuration Config) *RateLimiter { rl := &RateLimiter{ Quotas: make(map[string]ModelQuota), State: make(map[string]*UsageStats), } - applyConfig(rl, cfg) + applyConfig(rl, configuration) return rl } -func applyConfig(rl *RateLimiter, cfg Config) { +func applyConfig(rl *RateLimiter, configuration Config) { profiles := DefaultProfiles() - providers := cfg.Providers + providers := configuration.Providers - if len(providers) == 0 && len(cfg.Quotas) == 0 { + if len(providers) == 0 && len(configuration.Quotas) == 0 { providers = []Provider{ProviderGemini} } @@ -844,7 +844,7 @@ func applyConfig(rl *RateLimiter, cfg Config) { } } - maps.Copy(rl.Quotas, cfg.Quotas) + maps.Copy(rl.Quotas, configuration.Quotas) } func normaliseBackend(backend string) (string, error) { diff --git a/ratelimit_test.go b/ratelimit_test.go index 6dbb65c..5f0460a 100644 --- a/ratelimit_test.go +++ b/ratelimit_test.go @@ -1758,6 +1758,61 @@ func BenchmarkPersist(b *testing.B) { } } +// TestRatelimit_InvalidInputs_Bad covers error paths for invalid inputs to +// public API methods. +// +// rl.CanSend("model", -1) // returns false (negative tokens not allowed) +func TestRatelimit_InvalidInputs_Bad(t *testing.T) { + t.Run("CanSend with negative tokens returns false", func(t *testing.T) { + rl := newTestLimiter(t) + rl.Quotas["test-model"] = ModelQuota{MaxRPM: 10, MaxTPM: 1000, MaxRPD: 100} + assert.False(t, rl.CanSend("test-model", -1), "negative tokens should not be allowed") + }) + + t.Run("Decide with negative tokens returns DecisionInvalidTokens", func(t *testing.T) { + rl := newTestLimiter(t) + rl.Quotas["test-model"] = ModelQuota{MaxRPM: 10, MaxTPM: 1000, MaxRPD: 100} + decision := rl.Decide("test-model", -100) + assert.False(t, decision.Allowed) + assert.Equal(t, DecisionInvalidTokens, decision.Code) + assert.Contains(t, decision.Reason, "non-negative") + }) + + t.Run("WaitForCapacity with negative tokens returns error immediately", func(t *testing.T) { + rl := newTestLimiter(t) + err := rl.WaitForCapacity(context.Background(), "any-model", -1) + require.Error(t, err) + assert.Contains(t, err.Error(), "negative tokens") + }) + + t.Run("Load with corrupt YAML returns error", func(t *testing.T) { + tmpDir := t.TempDir() + path := testPath(tmpDir, "corrupt.yaml") + writeTestFile(t, path, "quotas: [this: is: not: valid") + rl, err := New() + require.NoError(t, err) + rl.filePath = path + err = rl.Load() + assert.Error(t, err, "corrupt YAML should return an error on Load") + }) + + t.Run("Persist with unwritable path returns error", func(t *testing.T) { + if isRootUser() { + t.Skip("chmod restrictions do not apply to root") + } + tmpDir := t.TempDir() + readonlyDir := testPath(tmpDir, "readonly") + ensureTestDir(t, readonlyDir) + setPathMode(t, readonlyDir, 0o555) + defer func() { _ = syscall.Chmod(readonlyDir, 0o755) }() + + rl := newTestLimiter(t) + rl.filePath = testPath(readonlyDir, "state.yaml") + err := rl.Persist() + assert.Error(t, err, "Persist should fail when directory is read-only") + }) +} + func TestRatelimit_EndToEndMultiProvider_Good(t *testing.T) { // Simulate a real-world scenario: limiter for both Gemini and Anthropic rl, err := NewWithConfig(Config{ diff --git a/sqlite.go b/sqlite.go index 984f7d4..d0a42ca 100644 --- a/sqlite.go +++ b/sqlite.go @@ -15,37 +15,41 @@ type sqliteStore struct { db *sql.DB } -// newSQLiteStore opens (or creates) a SQLite database at dbPath and initialises -// the schema. It follows the go-store pattern: single connection, WAL journal -// mode, and a 5-second busy timeout for contention handling. -func newSQLiteStore(dbPath string) (*sqliteStore, error) { - db, err := sql.Open("sqlite", dbPath) +// newSQLiteStore opens (or creates) a SQLite database at databasePath and +// initialises the schema. It follows the go-store pattern: single connection, +// WAL journal mode, and a 5-second busy timeout for contention handling. +// +// store, err := newSQLiteStore("/tmp/ratelimits.db") +func newSQLiteStore(databasePath string) (*sqliteStore, error) { + database, err := sql.Open("sqlite", databasePath) if err != nil { return nil, core.E("ratelimit.newSQLiteStore", "open", err) } // Single connection for PRAGMA consistency. - db.SetMaxOpenConns(1) + database.SetMaxOpenConns(1) - if _, err := db.Exec("PRAGMA journal_mode=WAL"); err != nil { - db.Close() + if _, err := database.Exec("PRAGMA journal_mode=WAL"); err != nil { + database.Close() return nil, core.E("ratelimit.newSQLiteStore", "WAL", err) } - if _, err := db.Exec("PRAGMA busy_timeout=5000"); err != nil { - db.Close() + if _, err := database.Exec("PRAGMA busy_timeout=5000"); err != nil { + database.Close() return nil, core.E("ratelimit.newSQLiteStore", "busy_timeout", err) } - if err := createSchema(db); err != nil { - db.Close() + if err := createSchema(database); err != nil { + database.Close() return nil, err } - return &sqliteStore{db: db}, nil + return &sqliteStore{db: database}, nil } // createSchema creates the tables and indices if they do not already exist. -func createSchema(db *sql.DB) error { +// +// if err := createSchema(database); err != nil { return nil, err } +func createSchema(database *sql.DB) error { stmts := []string{ `CREATE TABLE IF NOT EXISTS quotas ( model TEXT PRIMARY KEY, @@ -72,7 +76,7 @@ func createSchema(db *sql.DB) error { } for _, stmt := range stmts { - if _, err := db.Exec(stmt); err != nil { + if _, err := database.Exec(stmt); err != nil { return core.E("ratelimit.createSchema", "exec", err) } }