Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

📚 docs: Add Retry Addon documentation #3330

Merged
merged 9 commits into from
Feb 27, 2025

Conversation

grivera64
Copy link
Member

Description

Fiber added its first add-on in PR #1972, called retry. The documentation for this add-on is not currently included in v3's documentation on docs.gofiber.io.

This PR adds this missing documentation under a new collapsible tab under the middleware tab.

Fixes #3326

Changes introduced

  • Benchmarks: Describe any performance benchmarks and improvements related to the changes.
  • Documentation Update: Detail the updates made to the documentation and links to the changed files.
    • Add a new tab to docs.gofiber.io for addons
  • Changelog/What's New: Include a summary of the additions for the upcoming release notes.
  • Migration Guide: If necessary, provide a guide or steps for users to migrate their existing code to accommodate these changes.
  • API Alignment with Express: Explain how the changes align with the Express API.
  • API Longevity: Discuss the steps taken to ensure that the new or updated APIs are consistent and not prone to breaking changes.
  • Examples: Provide examples demonstrating the new features or changes in action.
    • Add a more detailed usage example of retry

Type of change

  • Documentation update (changes to documentation)

Checklist

Before you submit your pull request, please make sure you meet these requirements:

  • Followed the inspiration of the Express.js framework for new functionalities, making them similar in usage.
  • Conducted a self-review of the code and provided comments for complex or critical parts.
  • Updated the documentation in the /docs/ directory for Fiber's documentation.
  • Added or updated unit tests to validate the effectiveness of the changes or new features.
  • Ensured that new and existing unit tests pass locally with the changes.
  • Verified that any new dependencies are essential and have been agreed upon by the maintainers/community.
  • Aimed for optimal performance with minimal allocations in the new code.
  • Provided benchmarks for the new code to analyze and improve upon.

@grivera64 grivera64 self-assigned this Feb 26, 2025
@grivera64 grivera64 requested a review from a team as a code owner February 26, 2025 07:11
@grivera64 grivera64 requested review from gaby, sixcolors, ReneWerner87 and efectn and removed request for a team February 26, 2025 07:11
Copy link
Contributor

coderabbitai bot commented Feb 26, 2025

Walkthrough

This pull request modifies the NewExponentialBackoff function signature in the retry addon for Fiber, changing the parameter type to retry.Config and the return type to *retry.ExponentialBackoff. A new main function is added to demonstrate its usage, including retry logic for network requests. Additionally, the Config struct is updated by removing the currentInterval field. A new documentation file for the retry addon is introduced, detailing its functionality and configuration options. Lastly, a new section on addons is added to the Fiber documentation.

Changes

File(s) Summary of changes
addon/retry/README.md Updated NewExponentialBackoff function signature and return type, added a main function, removed currentInterval field from Config struct.
docs/addon/retry.md Introduced new documentation for the retry addon, detailing its functionality, providing function signatures, and defining the Config struct with default configurations.
docs/whats_new.md Added a new section "🔌 Addons" outlining the introduction of the Retry addon, including its functionality and example usage.

Sequence Diagram(s)

sequenceDiagram
    participant M as Main Function
    participant EB as ExponentialBackoff
    participant NR as Network Request

    M->>EB: Initialize ExponentialBackoff
    loop Retry Attempts
        M->>NR: Execute Network Request
        NR-->>M: Return response or error
        alt Failure (error or non-200)
            M->>EB: Wait using exponential backoff
        else Success (status 200)
            M-->>EB: Proceed with successful response
        end
    end
    M->>M: Panic if all retries fail
Loading

Suggested reviewers

  • sixcolors
  • gaby
  • ReneWerner87
  • efectn

Poem

Hop along, dear coder, in joyful delight,
For retries now dance in the code so bright.
Exponential leaps with each network quest,
Configs refined, putting errors to rest.
A rabbit sings of progress in every byte,
Celebrating changes from morning to night! 🐰


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

codecov bot commented Feb 26, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.21%. Comparing base (bc4c920) to head (ada57d2).
Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3330   +/-   ##
=======================================
  Coverage   84.21%   84.21%           
=======================================
  Files         116      116           
  Lines       11577    11577           
=======================================
  Hits         9749     9749           
  Misses       1397     1397           
  Partials      431      431           
Flag Coverage Δ
unittests 84.21% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🔭 Outside diff range comments (2)
addon/retry/README.md (2)

10-17: 🛠️ Refactor suggestion

Update TOC Link Fragment Consistency.
The Table of Contents includes the link Default Config Example, but no section exists with that exact anchor. To resolve the markdownlint error (MD051) and avoid confusing readers, please either update the link to match an existing heading (for example, Default Config) or rename the corresponding section heading to "Default Config Example."

🧰 Tools
🪛 LanguageTool

[duplication] ~15-~15: Possible typo: you repeated a word.
Context: ...ault Config](#default-config) - Custom Config - Config - [Default Config Example](#d...

(ENGLISH_WORD_REPEAT_RULE)

🪛 GitHub Check: markdownlint

[failure] 17-17: Link fragments should be valid
addon/retry/README.md:17:3 MD051/link-fragments Link fragments should be valid [Context: "Default Config Example"] https://github.com/DavidAnson/markdownlint/blob/v0.37.4/doc/md051.md

🪛 GitHub Actions: markdownlint

[error] 17-17: MD051/link-fragments Link fragments should be valid [Context: 'Default Config Example']


116-127: 🛠️ Refactor suggestion

Align Section Heading with TOC.
The section that declares the default configuration variable is titled "## Default Config," yet the Table of Contents lists a link for Default Config Example. For consistency and to fix the markdownlint error, please rename the section header (e.g., to "## Default Config Example") or adjust the TOC accordingly.

🧹 Nitpick comments (2)
addon/retry/README.md (1)

27-63: Clarify Example Code Variable Naming.
The example in the main function neatly illustrates how to use retry.NewExponentialBackoff. However, note that the local variable named client (declared inside the retry callback) shares its name with the imported package. To avoid any potential shadowing or confusion, consider renaming the local variable (for instance, to c or httpClient).

docs/addon/retry.md (1)

29-67: Refine Example Code for Consistency.
The provided example effectively demonstrates the usage of the retry addon. Similar to the README, the callback declares a local variable named client that might conflict with the imported package identifier. Consider renaming this local variable to improve clarity.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bc4c920 and 5fd4980.

⛔ Files ignored due to path filters (4)
  • docs/addon/_category_.json is excluded by !**/*.json
  • docs/client/_category_.json is excluded by !**/*.json
  • docs/extra/_category_.json is excluded by !**/*.json
  • docs/guide/_category_.json is excluded by !**/*.json
📒 Files selected for processing (2)
  • addon/retry/README.md (2 hunks)
  • docs/addon/retry.md (1 hunks)
🧰 Additional context used
🪛 GitHub Actions: markdownlint
addon/retry/README.md

[error] 17-17: MD051/link-fragments Link fragments should be valid [Context: 'Default Config Example']

🪛 LanguageTool
docs/addon/retry.md

[duplication] ~19-~19: Possible typo: you repeated a word.
Context: ...ault Config](#default-config) - Custom Config - Config - [Default Config Example](#d...

(ENGLISH_WORD_REPEAT_RULE)

🪛 GitHub Check: markdownlint
docs/addon/retry.md

[failure] 21-21: Link fragments should be valid
docs/addon/retry.md:21:3 MD051/link-fragments Link fragments should be valid [Context: "Default Config Example"] https://github.com/DavidAnson/markdownlint/blob/v0.37.4/doc/md051.md

⏰ Context from checks skipped due to timeout of 90000ms (8)
  • GitHub Check: unit (1.24.x, macos-13)
  • GitHub Check: unit (1.24.x, macos-latest)
  • GitHub Check: unit (1.24.x, windows-latest)
  • GitHub Check: unit (1.23.x, macos-latest)
  • GitHub Check: unit (1.23.x, windows-latest)
  • GitHub Check: Compare
  • GitHub Check: Analyse
  • GitHub Check: repeated

Comment on lines +12 to +21
## Table of Contents

- [Retry Addon](#retry-addon)
- [Table of Contents](#table-of-contents)
- [Signatures](#signatures)
- [Examples](#examples)
- [Default Config](#default-config)
- [Custom Config](#custom-config)
- [Config](#config)
- [Default Config Example](#default-config-example)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Ensure Valid TOC Links.
Within the Table of Contents, the link Default Config Example does not match any section heading in the document. To improve navigation and address markdownlint MD051, please update the link so that it corresponds to an existing anchor (for example, matching a header renamed to "## Default Config Example") or modify the TOC entry accordingly.

🧰 Tools
🪛 LanguageTool

[duplication] ~19-~19: Possible typo: you repeated a word.
Context: ...ault Config](#default-config) - Custom Config - Config - [Default Config Example](#d...

(ENGLISH_WORD_REPEAT_RULE)

🪛 GitHub Check: markdownlint

[failure] 21-21: Link fragments should be valid
docs/addon/retry.md:21:3 MD051/link-fragments Link fragments should be valid [Context: "Default Config Example"] https://github.com/DavidAnson/markdownlint/blob/v0.37.4/doc/md051.md

Comment on lines 120 to 131
## Default Config

```go
// DefaultConfig is the default config for retry.
var DefaultConfig = Config{
InitialInterval: 1 * time.Second,
MaxBackoffTime: 32 * time.Second,
Multiplier: 2.0,
MaxRetryCount: 10,
currentInterval: 1 * time.Second,
}
```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consistent Section Naming Recommended.
The section header for the default configuration variable is labeled "## Default Config" but is referenced in the TOC as Default Config Example. Renaming this header to "## Default Config Example" would ensure uniformity across the document and resolve the link fragment issue.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
addon/retry/README.md (1)

1-7: Improve Description Clarity in the Introduction
The introductory paragraph explains the addon’s purpose well, but the sentence “If all calls are failed, then, it returns an error.” can be rephrased for clarity. Consider updating it to:

-If all calls are failed, then, it returns an error.
+If all calls fail, an error is returned.

This change improves readability and grammatical accuracy.

docs/addon/retry.md (1)

1-11: Enhance Introduction Wording in YAML Documentation
The YAML front matter and introductory section effectively introduce the Retry Addon. However, similar to the README, consider rephrasing the sentence in the introduction to avoid repetition and improve clarity. For example:

-If all calls are failed, then, it returns an error. It adds a jitter at each retry step because adding
- a jitter is a way to break synchronization across the client and avoid collision.
+If all calls fail, an error is returned. A jitter is added at each retry step to break client synchronization and avoid collisions.

This revision removes redundant wording and makes the description more concise.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5fd4980 and 904549c.

📒 Files selected for processing (2)
  • addon/retry/README.md (2 hunks)
  • docs/addon/retry.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/addon/retry.md

[duplication] ~19-~19: Possible typo: you repeated a word.
Context: ...ault Config](#default-config) - Custom Config - Config - [Default Config Example](#d...

(ENGLISH_WORD_REPEAT_RULE)

⏰ Context from checks skipped due to timeout of 90000ms (7)
  • GitHub Check: unit (1.24.x, macos-13)
  • GitHub Check: unit (1.24.x, macos-latest)
  • GitHub Check: unit (1.24.x, windows-latest)
  • GitHub Check: unit (1.23.x, macos-13)
  • GitHub Check: unit (1.23.x, windows-latest)
  • GitHub Check: Compare
  • GitHub Check: repeated
🔇 Additional comments (5)
addon/retry/README.md (2)

8-18: Table of Contents Validation
The Table of Contents accurately lists all key sections, including “Default Config Example.” Ensure that the corresponding section header exactly matches the TOC entry so that in-page navigation works correctly. Currently, the TOC entry “Default Config Example” aligns with the later header, so no changes are necessary here.

🧰 Tools
🪛 LanguageTool

[duplication] ~15-~15: Possible typo: you repeated a word.
Context: ...ault Config](#default-config) - Custom Config - Config - [Default Config Example](#d...

(ENGLISH_WORD_REPEAT_RULE)


25-63: Examples Code Block – Clear and Comprehensive
The complete example in the “Examples” section demonstrates how to initialize the exponential backoff and perform a network request with retry logic. The inline comments make the code easy to follow. One consideration: while using panic(err) is acceptable in a demonstrative context, you might want to note (in comments) that in production code more graceful error handling could be preferable.

docs/addon/retry.md (3)

12-22: Table of Contents Consistency
The TOC accurately reflects the document’s sections, and the link “Default Config Example” now matches the header “## Default Config Example.” This resolves previous navigation issues and aligns with best practices for markdown links.

🧰 Tools
🪛 LanguageTool

[duplication] ~19-~19: Possible typo: you repeated a word.
Context: ...ault Config](#default-config) - Custom Config - Config - [Default Config Example](#d...

(ENGLISH_WORD_REPEAT_RULE)


29-67: Examples Code Block – Clear Demonstration
The “Examples” section provides a full, illustrative usage example of the retry functionality. The sample code is clear, well-commented, and effectively demonstrates how to handle errors during retries using the exponential backoff mechanism.


69-128: Configuration and Default Config Example Sections are Well Structured
The “Default Config,” “Custom Config,” “Config,” and “Default Config Example” sections comprehensively document the addon's configuration options. The code examples are consistent and maintain a clear style throughout the document. These sections offer valuable, easily digestible information for users looking to customize their retry behavior.

Copy link
Member

@ReneWerner87 ReneWerner87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ReneWerner87 ReneWerner87 added this to the v3 milestone Feb 26, 2025
@gaby gaby changed the title 📚 Docs: Add Addon docs 📚 docs: Add Retry Addon documentation Feb 26, 2025
@ReneWerner87
Copy link
Member

@grivera64 can you check our hints

@grivera64
Copy link
Member Author

@grivera64 can you check our hints

@ReneWerner87 For sure, I've made the changes based on the hints.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (4)
addon/retry/README.md (2)

27-62: Example Main Function – Avoid Variable Shadowing
The example code clearly demonstrates how to initialize the exponential backoff and use the retry mechanism. However, in the anonymous function (line 45), the local variable client is declared using client := client.New(), which shadows the imported package named client. This shadowing can lead to confusion and potential maintenance issues. Consider renaming the local variable (for example, to httpClient) to improve clarity.

-        client := client.New()
+        httpClient := client.New()

84-92: Default Config Example – Document Only Public Fields
The "Default Config Example" section provides a clear default configuration; however, the field currentInterval (line 90) appears to be an internal state used for tracking purposes and is not exported (note the lowercase first letter). If this field is not intended for external configuration, consider omitting it from the public documentation to avoid confusion for users.

docs/addon/retry.md (2)

31-66: Example Main Function – Avoid Variable Shadowing
Similar to the README file, the example main function here (lines 31–66) is effective in demonstrating the usage of the retry addon. However, note the same issue on line 49 where client := client.New() is used. Renaming this local variable (for example, to httpClient) would avoid shadowing the imported package name and clarify the intent.

-        client := client.New()
+        httpClient := client.New()

85-96: Default Config Example – Expose Only Public Configuration
In the "Default Config Example" section, the DefaultConfig variable is clearly defined. However, the inclusion of the field currentInterval (line 94) might be confusing if it is considered internal state. Since it is not exported (due to the lowercase first letter), consider whether it should be documented for public use. If it is not intended for users to modify or rely on, you might remove it from the example or add a clarifying note.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 904549c and d280200.

⛔ Files ignored due to path filters (1)
  • docs/addon/_category_.json is excluded by !**/*.json
📒 Files selected for processing (2)
  • addon/retry/README.md (1 hunks)
  • docs/addon/retry.md (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (9)
  • GitHub Check: unit (1.24.x, macos-13)
  • GitHub Check: unit (1.24.x, macos-latest)
  • GitHub Check: unit (1.24.x, windows-latest)
  • GitHub Check: unit (1.23.x, macos-13)
  • GitHub Check: unit (1.23.x, macos-latest)
  • GitHub Check: unit (1.23.x, windows-latest)
  • GitHub Check: Compare
  • GitHub Check: Analyse
  • GitHub Check: repeated
🔇 Additional comments (2)
docs/addon/retry.md (2)

1-3: Front Matter Verification
The front matter (lines 1–3) is present and properly formatted for identifying the document. No changes needed.


12-21: Table of Contents – Verify Anchor Consistency
The Table of Contents lists several section links (including "Default Config Example"). Make sure that each link exactly matches the corresponding header (e.g. the header at line 85 is "## Default Config Example") so that users can navigate the document correctly.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🔭 Outside diff range comments (2)
addon/retry/README.md (1)

111-123: ⚠️ Potential issue

Inconsistent DefaultConfig Definition:
The DefaultConfig variable includes an unrecognized field currentInterval, which is not defined in the Config struct above. This inconsistency may lead to confusion or compile errors. Consider removing the currentInterval field to align with the struct definition.

A suggested diff:

 var DefaultConfig = Config{
     InitialInterval: 1 * time.Second,
     MaxBackoffTime:  32 * time.Second,
     Multiplier:      2.0,
     MaxRetryCount:   10,
-    currentInterval: 1 * time.Second,
 }
docs/addon/retry.md (1)

115-127: ⚠️ Potential issue

Inconsistent DefaultConfig Definition:
Similar to the README file, the DefaultConfig variable here includes the currentInterval field, which is not part of the defined Config struct. Removing this field will prevent potential confusion and ensure consistency between the struct and its default initialization.

A suggested diff:

 var DefaultConfig = Config{
     InitialInterval: 1 * time.Second,
     MaxBackoffTime:  32 * time.Second,
     Multiplier:      2.0,
     MaxRetryCount:   10,
-    currentInterval: 1 * time.Second,
 }
🧹 Nitpick comments (2)
addon/retry/README.md (1)

8-17: Ensure Valid TOC Links:
The Table of Contents is comprehensive. Please verify that every anchor link (e.g., “#default-config”, “#default-config-example”) exactly matches its corresponding section header. Also, consider whether the separation between "Default Config" (usage example) and "Default Config Example" (detailed variable declaration) is clear and necessary.

🧰 Tools
🪛 LanguageTool

[duplication] ~15-~15: Possible typo: you repeated a word.
Context: ...ault Config](#default-config) - Custom Config - Config - [Default Config Example](#d...

(ENGLISH_WORD_REPEAT_RULE)

docs/addon/retry.md (1)

12-22: Valid TOC and Section Consistency:
The Table of Contents lists all the key sections. Ensure that each TOC link correctly corresponds to the section headers and consider clarifying the distinction between “Default Config” (a simple usage example) and “Default Config Example” (the detailed default variable declaration).

🧰 Tools
🪛 LanguageTool

[duplication] ~19-~19: Possible typo: you repeated a word.
Context: ...ault Config](#default-config) - Custom Config - Config - [Default Config Example](#d...

(ENGLISH_WORD_REPEAT_RULE)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d280200 and 842a05b.

⛔ Files ignored due to path filters (1)
  • docs/addon/_category_.json is excluded by !**/*.json
📒 Files selected for processing (2)
  • addon/retry/README.md (2 hunks)
  • docs/addon/retry.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/addon/retry.md

[duplication] ~19-~19: Possible typo: you repeated a word.
Context: ...ault Config](#default-config) - Custom Config - Config - [Default Config Example](#d...

(ENGLISH_WORD_REPEAT_RULE)

⏰ Context from checks skipped due to timeout of 90000ms (6)
  • GitHub Check: unit (1.24.x, macos-13)
  • GitHub Check: unit (1.24.x, macos-latest)
  • GitHub Check: unit (1.24.x, windows-latest)
  • GitHub Check: unit (1.23.x, windows-latest)
  • GitHub Check: repeated
  • GitHub Check: Compare
🔇 Additional comments (13)
addon/retry/README.md (6)

1-7: Clear Introduction:
The introduction succinctly explains what the Retry Addon does and its intended mechanism using exponential backoff with jitter. This sets the right context for users.


19-23: Clear API Signature:
The function signature for NewExponentialBackoff is presented clearly, making it easy for users to understand how to initialize the addon.


25-63: Effective Usage Example:
The main function example demonstrates the retry mechanism very well, including proper error handling and retry logic. The overall flow is easy to follow.


65-69: Default Configuration Call:
The simple call retry.NewExponentialBackoff() effectively shows how the addon is used with default configuration settings.


71-80: Custom Config Usage:
The custom configuration example clearly demonstrates how to override default parameters, providing explicit values for timing, multiplier, and retry count.


82-110: Comprehensive Config Struct:
The Config struct is well documented with each field’s description and default value, which is very helpful for users seeking to customize their configuration.

docs/addon/retry.md (7)

1-4: Front Matter is Correct:
The YAML front matter (including id: retry) is properly set up for this documentation page.


5-11: Clear Introduction:
The header and introductory text clearly explain the purpose and functionality of the Retry Addon, making its role and behavior easy to understand.


23-28: Clear Function Signature:
The NewExponentialBackoff function signature is displayed clearly within a code block, effectively communicating the API.


29-67: Effective Retry Example:
The example provided in the main function clearly demonstrates how to use the retry mechanism, including proper error handling and output messaging.


69-73: Default Config Usage Example:
The snippet that calls retry.NewExponentialBackoff() with no arguments succinctly shows the default behavior of the addon.


75-84: Custom Config Example:
The custom configuration example is well-crafted and illustrates how to override default parameters effectively.


86-113: Well-Documented Config Struct:
The Config struct is thoroughly detailed with explanations for each field and their default values, aiding user comprehension of the available configuration options.

@ReneWerner87
Copy link
Member

ReneWerner87 commented Feb 27, 2025

@grivera64

@grivera64
Copy link
Member Author

grivera64 commented Feb 27, 2025

@grivera64

Done, sorry about missing those hints the first time around. I've added a commit to add retry to whats_new.md.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🔭 Outside diff range comments (2)
addon/retry/README.md (1)

115-122: ⚠️ Potential issue

Critical Issue: Remove Deprecated Field from Default Config
The "Default Config Example" block (lines 115–122) still includes a value for currentInterval, which has been removed from the Config struct. This leads to inconsistency and could confuse users. Please remove the line setting currentInterval so that the default configuration only assigns the documented fields. For example:

 var DefaultConfig = Config{
-    InitialInterval: 1 * time.Second,
-    MaxBackoffTime:  32 * time.Second,
-    Multiplier:      2.0,
-    MaxRetryCount:   10,
-    currentInterval: 1 * time.Second,
+    InitialInterval: 1 * time.Second,
+    MaxBackoffTime:  32 * time.Second,
+    Multiplier:      2.0,
+    MaxRetryCount:   10,
 }
docs/addon/retry.md (1)

117-127: ⚠️ Potential issue

Critical Issue: Remove Deprecated Field from Default Config Example
Similar to the issue in addon/retry/README.md, the "Default Config Example" in this document (lines 117–127) includes the currentInterval field. Please remove it so that the DefaultConfig instance only includes the fields defined in the Config struct. For example:

 var DefaultConfig = Config{
-    InitialInterval: 1 * time.Second,
-    MaxBackoffTime:  32 * time.Second,
-    Multiplier:      2.0,
-    MaxRetryCount:   10,
-    currentInterval: 1 * time.Second,
+    InitialInterval: 1 * time.Second,
+    MaxBackoffTime:  32 * time.Second,
+    Multiplier:      2.0,
+    MaxRetryCount:   10,
 }

This adjustment will ensure consistency with the updated Config struct.

🧹 Nitpick comments (3)
docs/whats_new.md (2)

36-37: TOC Update: Verify Anchor Link Validity
The addition of the "🔌 Addons" entry in the Table of Contents (line 36) is clear. However, please verify that the anchor "#-addons" (if that is generated from the header) correctly corresponds to the "## 🔌 Addons" section later in the document. Also, note the pipeline error regarding the "📋 Migration guide" link at line 37; ensure that the anchor in that link exactly matches the corresponding header.

🧰 Tools
🪛 markdownlint-cli2 (0.17.2)

37-37: Link fragments should be valid
null

(MD051, link-fragments)

🪛 GitHub Check: markdownlint

[failure] 37-37: Link fragments should be valid
docs/whats_new.md:37:3 MD051/link-fragments Link fragments should be valid [Context: "📋 Migration guide"] https://github.com/DavidAnson/markdownlint/blob/v0.37.4/doc/md051.md

🪛 GitHub Actions: markdownlint

[error] 37-37: MD051/link-fragments Link fragments should be valid [Context: '📋 Migration guide']


943-995: Content Clarity: Refine Retry Addon Description
The new "Retry" addon section (lines 943–995) provides a detailed explanation and usage example, which is great. Consider refining some language for clarity. For instance, rephrase “calls the function multiple times and tries to make it successful. If all calls are failed, then, it returns an error” to “calls the function repeatedly until it succeeds; if all attempts fail, it returns an error.” This small change improves readability without altering the technical details.

addon/retry/README.md (1)

68-84: Example Code Review: Clarity and Completeness
The examples provided for both default and custom configurations are clear and practical. Ensure that users know to import "time" when using these examples, as time-based constants are used in the configuration. This is a minor documentation detail that could prevent confusion for new users.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 842a05b and c71fb21.

📒 Files selected for processing (3)
  • addon/retry/README.md (2 hunks)
  • docs/addon/retry.md (1 hunks)
  • docs/whats_new.md (2 hunks)
🧰 Additional context used
🪛 GitHub Actions: markdownlint
docs/whats_new.md

[error] 37-37: MD051/link-fragments Link fragments should be valid [Context: '📋 Migration guide']

🪛 LanguageTool
docs/addon/retry.md

[duplication] ~19-~19: Possible typo: you repeated a word.
Context: ...ault Config](#default-config) - Custom Config - Config - [Default Config Example](#d...

(ENGLISH_WORD_REPEAT_RULE)

⏰ Context from checks skipped due to timeout of 90000ms (5)
  • GitHub Check: unit (1.24.x, windows-latest)
  • GitHub Check: unit (1.23.x, macos-13)
  • GitHub Check: unit (1.23.x, windows-latest)
  • GitHub Check: repeated
  • GitHub Check: Compare
🔇 Additional comments (3)
addon/retry/README.md (1)

26-27: Signature Update is Correct
The updated function signature

func NewExponentialBackoff(config ...retry.Config) *retry.ExponentialBackoff

accurately reflects the changes described in the PR objectives. Good job updating the documentation accordingly.

docs/addon/retry.md (2)

1-7: Front Matter and Title: Looks Good
The front matter along with the title and introductory description clearly sets the context for the Retry Addon documentation. The added ID (id: retry) will help in link generation.


23-28: TOC and Section Alignment
The Table of Contents in this file is comprehensive. The "Default Config Example" entry matches the section header in line 115, which resolves earlier concerns from previous reviews. Great work aligning the section names.

@ReneWerner87
Copy link
Member

@grivera64 can you check the lint errors ?

@grivera64
Copy link
Member Author

@grivera64 can you check the lint errors ?

Fixed!

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
docs/whats_new.md (1)

943-995: Retry Addon Documentation Clarity and Style
The new "Retry" addon section is well organized and provides a clear example of how to use the feature. To further enhance clarity and readability, consider the following suggestions:

  • Grammar Improvement:
    Instead of:
    -It calls the function multiple times and tries to make it successful. If all calls are failed, then, it returns an error.
    +It repeatedly calls the function until it succeeds; if all retry attempts fail, it returns an error.
  • Jitter Explanation:
    Consider rephrasing the sentence about jitter for clarity. For example,
    "It adds a jitter to each retry attempt to prevent clients from synchronizing their retries, thus reducing the risk of collision."

Overall, these minor refinements will make the documentation even clearer for users.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c71fb21 and ada57d2.

📒 Files selected for processing (1)
  • docs/whats_new.md (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
  • GitHub Check: unit (1.24.x, windows-latest)
  • GitHub Check: unit (1.23.x, windows-latest)
  • GitHub Check: Compare
  • GitHub Check: repeated
🔇 Additional comments (1)
docs/whats_new.md (1)

36-36: New Navigation Entry for Addons
Adding the "🔌 Addons" navigation link to the table of contents is a welcome update and is consistent with the structure of the other sections. Please double-check that the anchor (#-addons) correctly maps to the new "Addons" section below.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
docs/whats_new.md (2)

943-952: New Addons Documentation Section
The documentation for Addons—and specifically for the Retry addon—is comprehensive and well organized. The description explains the purpose of the Retry addon and its basic working mechanism.

Suggestions:

  • Consider refining a couple of phrases for clarity. For example, change “tries to make it successful. If all calls are failed, then, it returns an error.” to “tries to succeed, and if all attempts fail, it returns an error.”
  • Remove the extraneous comma after “then” for a smoother read.

953-995: Clear and Practical Retry Example
The example code clearly demonstrates how to use the Retry addon with an exponential backoff. It effectively illustrates the creation of a backoff instance, usage of a retry loop, and error handling upon an unsuccessful network request.

Suggestions:

  • Avoid Shadowing the Package Name: In the retry callback, the local variable named client shadows the imported package name. To improve clarity and avoid potential confusion, consider renaming this local variable. For example:
-    err = expBackoff.Retry(func() error {
-        client := client.New()
-        resp, err = client.Get("https://gofiber.io")
+    err = expBackoff.Retry(func() error {
+        httpClient := client.New()
+        resp, err = httpClient.Get("https://gofiber.io")

This change will prevent the inadvertent shadowing of the package name client and enhance code readability.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c71fb21 and ada57d2.

📒 Files selected for processing (1)
  • docs/whats_new.md (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: unit (1.23.x, windows-latest)
  • GitHub Check: Compare
  • GitHub Check: repeated
🔇 Additional comments (1)
docs/whats_new.md (1)

36-38: New TOC Entry for Addons
The addition of the "🔌 Addons" entry to the table of contents nicely reflects the newly introduced section below. This update improves navigability and ensures that users can quickly find information on the new Retry addon.

@ReneWerner87 ReneWerner87 merged commit b568915 into gofiber:main Feb 27, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

🧹 [Maintenance]: Add missing retry add-on documentation to docs.gofiber.io
3 participants