fix: rate-limit retry loop and docstring coverage#559
Conversation
…ings Replace the deeply nested else-if chain for rate-limit retries in the live stream tick loop with a clean retry loop with backoff. The old structure also silently swallowed non-rate-limit errors on the first attempt. Add doc comments to all functions and types missing them across streamdl.go, download_stream.go, grpc_client.go, config.go, config_reader.go, and move_file.go. Closes #555 Closes #554
📝 WalkthroughWalkthroughThis PR improves docstring coverage across multiple Go source files and fixes a control flow bug in the rate-limit retry logic for live streams. Documentation comments are added to exported types and functions in config, download, gRPC client, and file move modules. The rate-limit retry logic in streamdl.go is refactored from nested conditionals to an explicit loop-based approach with exponential backoff. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
streamdl.go (1)
201-204: Avoid string-matching errors for retry control.Lines 133 and 201 rely on exact message text (
"rate limited"), which is brittle. Prefer a shared sentinel error anderrors.Isso retry behavior stays stable if message wording changes.♻️ Proposed hardening (cross-file)
--- a/grpc_client.go +++ b/grpc_client.go @@ +var ErrRateLimited = errors.New("rate limited") @@ - case codes.ResourceExhausted: - return nil, errors.New("rate limited") + case codes.ResourceExhausted: + return nil, ErrRateLimited @@ - case codes.ResourceExhausted: - return "", errors.New("rate limited") + case codes.ResourceExhausted: + return "", ErrRateLimited--- a/streamdl.go +++ b/streamdl.go @@ import ( + "errors" "flag" @@ - if err.Error() == "rate limited" { + if errors.Is(err, ErrRateLimited) { log.Errorf("Rate limited checking VODs for %s, skipping", streamer.User) // ... } else { @@ - if err.Error() != "rate limited" { + if !errors.Is(err, ErrRateLimited) { log.Warnf("GetStream failed for user=%s: %v", streamer.User, err) break }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@streamdl.go` around lines 201 - 204, Replace the brittle string comparison of err.Error() == "rate limited" with a package-level sentinel error (e.g., var ErrRateLimited = errors.New("rate limited")) and use errors.Is to check it; update the failing branch in the code around GetStream (and the similar check around line 133) to use if !errors.Is(err, ErrRateLimited) { ... } and ensure wherever the rate-limited condition is created or returned it wraps or returns ErrRateLimited (using fmt.Errorf("%w", ErrRateLimited) or returning ErrRateLimited directly) so the errors.Is check reliably detects the sentinel.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@streamdl.go`:
- Around line 201-204: Replace the brittle string comparison of err.Error() ==
"rate limited" with a package-level sentinel error (e.g., var ErrRateLimited =
errors.New("rate limited")) and use errors.Is to check it; update the failing
branch in the code around GetStream (and the similar check around line 133) to
use if !errors.Is(err, ErrRateLimited) { ... } and ensure wherever the
rate-limited condition is created or returned it wraps or returns ErrRateLimited
(using fmt.Errorf("%w", ErrRateLimited) or returning ErrRateLimited directly) so
the errors.Is check reliably detects the sentinel.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 64c73079-3288-4a4a-99ea-099fd022df7a
📒 Files selected for processing (6)
config.goconfig_reader.godownload_stream.gogrpc_client.gomove_file.gostreamdl.go
Summary
getStreamcall — now all error paths are handled.streamdl.go,download_stream.go,grpc_client.go,config.go,config_reader.go, andmove_file.go. Python server already had full coverage.Closes #555
Closes #554
Test plan
go build ./...passesgo test ./...passesgetStreamto return "rate limited")Summary by CodeRabbit
Documentation
Bug Fixes