docs: workers howto for jitter and middleware chain#77
Conversation
Add Jitter, Middleware, and Built-in Middleware sections. Update all code examples for new API: NewWorker(name).HandlerFunc(fn), info.Name(), info.Attempt() Update WorkerContext → WorkerInfo with private fields + getters. Add Playwright content test for middleware/jitter keywords. Update Packages.md description. Ref: go-coldbrew/workers#5
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 8 minutes and 22 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughDocumentation replaces the WorkerContext-based handler model with cycle handlers taking context.Context and *WorkerInfo, introduces a middleware/interceptor architecture with built-in middleware and jitter/initial-delay options for periodic workers, updates logging/metrics guidance, and adds an E2E test for the workers how‑to page. Changes
Sequence Diagram(s)sequenceDiagram
participant Supervisor
participant Runner
participant MiddlewareChain
participant Handler
participant LockService
participant Metrics
Supervisor->>Runner: start worker
Runner->>MiddlewareChain: invoke cycle (context, *WorkerInfo)
MiddlewareChain->>LockService: acquire distributed lock (optional)
MiddlewareChain->>Handler: call CycleHandler
Handler-->>MiddlewareChain: return result / ErrDoNotRestart
MiddlewareChain->>Metrics: record attempt duration
Runner->>Supervisor: report status / decide restart
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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 unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Updates the Workers documentation to reflect the new go-coldbrew/workers API (middleware/interceptor chain, jitter, and WorkerInfo), and adds a Playwright content regression test to ensure the updated page renders key new concepts.
Changes:
- Expanded
howto/workers.mdwith new sections for jitter, middleware types/APIs, and built-in middleware; updated examples to the new handler/WorkerInfoAPI. - Updated
Packages.mdWorkers entry to mention middleware and jitter support. - Added a Playwright content test asserting the Workers howto renders key middleware/jitter terms and code blocks.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/content.spec.ts | Adds a content regression test for the Workers howto page. |
| howto/workers.md | Major docs rewrite: new jitter + middleware sections and updated examples/types. |
| Packages.md | Updates Workers package description to include middleware + jitter. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… add ErrDoNotRestart
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…g, fix WorkerInfo listing
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@howto/workers.md`:
- Around line 378-383: The fenced code block closing fence is glued to the
trailing "))" in the workers examples, e.g. the snippet using
workers.NewWorker("metrics-reporter").HandlerFunc(workers.EveryInterval(...
func(ctx context.Context, info *workers.WorkerInfo) error { return
reportMetrics(ctx) }, )), so move the closing triple-backticks onto its own line
after the closing parenthesis; apply the same fix to the other four occurrences
with the same pattern (the code blocks that end with EveryInterval/HandlerFunc
and reportMetrics-like handlers) so each block ends with the final "),\n))\n```"
sequence (closing fence on a separate line).
- Around line 147-150: The fenced pseudocode block containing the lines "spread
= base × percent ÷ 100" and "jittered = base − spread + rand(2 × spread)" is
missing a language tag; update that code fence to include a language (use "text"
or "pseudo") so markdownlint MD040 is satisfied (e.g., change the opening ``` to
```text).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4e54aa08-356f-4665-835f-27fa5f9eb5a2
📒 Files selected for processing (3)
Packages.mdhowto/workers.mdtests/content.spec.ts
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
howto/workers.md (1)
147-150:⚠️ Potential issue | 🟡 MinorAdd a language to this fenced code block (MD040).
The formula block is still missing a fence language, which keeps markdownlint warning active.
Suggested fix
-``` +```text spread = base × percent ÷ 100 jittered = base − spread + rand(2 × spread)</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@howto/workers.mdaround lines 147 - 150, The fenced code block containing
the formulas "spread = base × percent ÷ 100" and "jittered = base − spread +
rand(2 × spread)" is missing a fence language which triggers markdownlint MD040;
update the fence to include a language token (for exampletext) so the block
becomestext ...to silence MD040 while preserving the formula content.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In@howto/workers.md:
- Line 152: Update the text in howto/workers.md where the interval description
reads "1ms" to use a space between number and unit ("1 ms") for consistent unit
formatting; locate the sentence containing "The effective interval is clamped to
a minimum of 1ms (never zero or negative)." and change "1ms" to "1 ms".
Duplicate comments:
In@howto/workers.md:
- Around line 147-150: The fenced code block containing the formulas "spread =
base × percent ÷ 100" and "jittered = base − spread + rand(2 × spread)" is
missing a fence language which triggers markdownlint MD040; update the fence to
include a language token (for exampletext) so the block becomes ```text ...🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
- Add "Why Workers" section with plain goroutines comparison + distributed locking - Add "Handler Return Values" section documenting return nil/error/ErrDoNotRestart behavior - Add "Graceful Shutdown" section - Add "Testing" section with NewWorkerInfo, WithTestChildren, RunWorker patterns - Add "Best Practices" section - Fix Builder Methods table: add WithMetrics, fix parameter types, fix Slog description - Fix log import ambiguity in examples - Fix RunWorker docs (discards error) - Clarify two logging layers (supervisor slog vs middleware Slog) - Label ColdBrew Integration as planned
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| | `return nil` | Worker stops permanently (even with restart enabled) | | ||
| | `return error` | Worker restarts with backoff (if restart enabled) | | ||
| | `return ctx.Err()` | Clean shutdown — context was cancelled | | ||
| | `return workers.ErrDoNotRestart` | Explicit permanent stop (e.g., channel closed, work exhausted) | |
There was a problem hiding this comment.
The “Handler Return Values” table says return nil stops the worker permanently, but a few lines later the doc says periodic workers (with Every) should return nil for success and continue to the next tick. This is internally inconsistent and will confuse readers. Consider updating the table to distinguish long-running vs periodic behavior (or clarify that nil means “cycle succeeded” for periodic workers but “stop” for non-periodic workers).
| | `return nil` | Worker stops permanently (even with restart enabled) | | |
| | `return error` | Worker restarts with backoff (if restart enabled) | | |
| | `return ctx.Err()` | Clean shutdown — context was cancelled | | |
| | `return workers.ErrDoNotRestart` | Explicit permanent stop (e.g., channel closed, work exhausted) | | |
| | `return nil` | **Long-running workers**: stop permanently if the handler returns. **Periodic workers** (with `Every`): mark the cycle successful and wait for the next tick. | | |
| | `return error` | Treat the run/cycle as failed and restart with backoff (if restart enabled). | | |
| | `return ctx.Err()` | Clean shutdown — the worker context was cancelled. | | |
| | `return workers.ErrDoNotRestart` | Explicit permanent stop (e.g., channel closed, work exhausted). | |
There was a problem hiding this comment.
Fixed — table now distinguishes long-running vs periodic workers for return nil behavior.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
howto/workers.md (1)
201-201:⚠️ Potential issue | 🟡 MinorUse
1 ms(with a space) for unit formatting consistency.Line 201 still says
1ms; please change it to1 msto match standard technical writing style used elsewhere.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@howto/workers.md` at line 201, Update the unit formatting in the sentence currently reading "The effective interval is clamped to a minimum of 1ms (never zero or negative)." to use a space between the number and unit so it reads "The effective interval is clamped to a minimum of 1 ms (never zero or negative).", ensuring consistency with other instances of "ms" formatting in the document.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@howto/workers.md`:
- Line 201: Update the unit formatting in the sentence currently reading "The
effective interval is clamped to a minimum of 1ms (never zero or negative)." to
use a space between the number and unit so it reads "The effective interval is
clamped to a minimum of 1 ms (never zero or negative).", ensuring consistency
with other instances of "ms" formatting in the document.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
howto/workers.md (1)
201-201:⚠️ Potential issue | 🟡 MinorUse
1 msinstead of1msfor unit formatting consistency.LanguageTool flags this as a spelling/orthography issue. Standard technical writing style uses a space between number and unit.
-The effective interval is clamped to a minimum of 1ms (never zero or negative). Each tick recomputes independently — successive intervals differ. +The effective interval is clamped to a minimum of 1 ms (never zero or negative). Each tick recomputes independently — successive intervals differ.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@howto/workers.md` at line 201, Replace the unit formatting "1ms" with "1 ms" in the sentence inside howto/workers.md (the sentence: "The effective interval is clamped to a minimum of 1ms (never zero or negative). Each tick recomputes independently — successive intervals differ."); update it so the number and unit are separated by a space ("1 ms") to match standard technical style.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@howto/workers.md`:
- Line 201: Replace the unit formatting "1ms" with "1 ms" in the sentence inside
howto/workers.md (the sentence: "The effective interval is clamped to a minimum
of 1ms (never zero or negative). Each tick recomputes independently — successive
intervals differ."); update it so the number and unit are separated by a space
("1 ms") to match standard technical style.
- Fix WithBackoffJitter type (interface, not function) - Add go-coldbrew/log import comments to disambiguate examples - Fix Slog section to say "log lines" (plural) with LogContext pairing - Move ErrDoNotRestart section to after Handler Return Values - Remove misleading errors.Is example (Run doesn't propagate ErrDoNotRestart) - Remove redundant WithRestart(true) in per-tenant example - Fix log.Fatal → slog.Error in Prometheus example - Update overview diagram to show middleware → handler - Add complete CycleHandler struct example with Close()
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1. batchProcessor one-liner uses NewBatchProcessor constructor 2. "Before" goroutine example shows ctx creation 3. EveryInterval section: builder form first, manual form second 4. EveryInterval manual form uses named function (not inline) 5. Per-worker metrics example uses named handler not undefined fn
… conn consistently
…in CycleHandler section
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… middleware not core
Summary
WithJitter(percent),WithDefaultJitter(percent),WithInitialDelay, formulaCycleHandler,CycleFunc,Middlewaretypes, worker-level and run-level APIs, writing custom middlewareNewWorker(name).HandlerFunc(fn),info.Name(), etc.)WorkerContext→WorkerInfowith private fields + gettersWithJitter,Middleware,CycleHandler,CycleFunc,DistributedLockDepends on go-coldbrew/workers#5
Test plan
npx playwright testpasses with local Jekyll serverSummary by CodeRabbit