Skip to content

🧹 chore: Simplify HealthCheck middleware#3380

Merged
ReneWerner87 merged 2 commits intomainfrom
update-healthcheck-mw
Mar 30, 2025
Merged

🧹 chore: Simplify HealthCheck middleware#3380
ReneWerner87 merged 2 commits intomainfrom
update-healthcheck-mw

Conversation

@gaby
Copy link
Member

@gaby gaby commented Mar 29, 2025

Description

  • Simplify usage of middleware

Changes introduced

  • Documentation Update: Detail the updates made to the documentation and links to the changed files.
  • API Longevity: Discuss the steps taken to ensure that the new or updated APIs are consistent and not prone to breaking changes.

@gaby gaby added this to the v3 milestone Mar 29, 2025
@gaby gaby requested a review from Copilot March 29, 2025 19:59
@gaby gaby requested a review from a team as a code owner March 29, 2025 19:59
@gaby gaby added this to v3 Mar 29, 2025
@gaby gaby requested review from ReneWerner87, efectn and sixcolors and removed request for a team March 29, 2025 19:59
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 29, 2025

Walkthrough

This pull request refactors the health check middleware in the Fiber framework. It renames the constructor function from NewHealthChecker to New, updates the function signatures, and changes the Config.Probe field from a custom HealthChecker type to a function signature (func(fiber.Ctx) bool). The default probe function and default configuration helper are also updated. Documentation and test cases have been revised accordingly to reflect these changes.

Changes

File(s) Change Summary
docs/.../healthcheck.md, docs/.../whats_new.md Renamed NewHealthChecker to New; updated example calls and function signatures in documentation.
middleware/healthcheck/config.go Changed Config.Probe from HealthChecker to func(fiber.Ctx) bool; renamed defaultConfigV3 to defaultConfig and updated the default probe function’s parameter name.
middleware/healthcheck/healthcheck.go, middleware/healthcheck/healthcheck_test.go Removed the HealthChecker type; renamed NewHealthChecker to New and updated related test cases accordingly.

Possibly related PRs

Suggested labels

📒 Documentation

Suggested reviewers

  • sixcolors
  • efectn
  • ReneWerner87

Poem

I'm a bunny with a code stash so neat,
Hopping through changes with nimble feet.
Renamed functions and cleaner ways,
Lighting up Fiber in joyful displays.
Celebrate refactoring with a happy beat! 🐇✨


📜 Recent review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between a6572af and 8f5b5f4.

📒 Files selected for processing (4)
  • docs/middleware/healthcheck.md (5 hunks)
  • docs/whats_new.md (1 hunks)
  • middleware/healthcheck/config.go (1 hunks)
  • middleware/healthcheck/healthcheck_test.go (8 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • docs/whats_new.md
  • middleware/healthcheck/config.go
⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: Compare
  • GitHub Check: unit (1.23.x, windows-latest)
  • GitHub Check: repeated
🔇 Additional comments (13)
middleware/healthcheck/healthcheck_test.go (7)

37-39: Constructor function rename looks good

The changes from NewHealthChecker() to New() align with the PR objective to simplify the HealthCheck middleware. This is a common Go idiom where the constructor is simply named New.


56-58: Consistent constructor use

The changes correctly apply the new constructor name across test cases, maintaining consistency throughout the codebase.


76-95: Proper constructor use with custom configurations

The changes correctly update the constructor calls while preserving the custom probe function configurations. The simplification makes the code more readable without changing behavior.


126-140: Consistent middleware configuration

The changes properly apply the constructor rename in tests with nested path configurations, maintaining the expected behavior.


167-175: Correctly updated Next configuration

The changes correctly implement the new constructor for the Next configuration case, ensuring backward compatibility with existing middleware chaining behavior.


187-189: Benchmark updated successfully

The benchmark functions have been correctly updated to use the new constructor name, ensuring that performance tests remain valid.


209-211: Parallel benchmark updated successfully

The parallel benchmark tests have been correctly updated to use the new constructor, maintaining test coverage for concurrent scenarios.

docs/middleware/healthcheck.md (6)

30-30: Simplified function signature

Updated function signature from NewHealthChecker to New follows Go conventions for constructor naming and simplifies API usage.


48-55: Simplified API examples

The examples have been updated to reflect the new API, making it clearer and more concise for users. This improves the documentation's readability.


60-79: Updated examples with custom configurations

The examples with custom configurations now use the simplified constructor name while maintaining the same functionality, ensuring consistency across the documentation.


90-94: Updated app.All example

The app.All example has been correctly updated with the new API, preserving important documentation about method handling behavior.


117-117: Simplified Probe field type

The Probe field type has been changed from a custom HealthChecker type to a more straightforward function signature func(fiber.Ctx) bool, which simplifies the API and makes it more intuitive for users.


126-126: Updated defaultProbe signature

The default probe function now uses the _ parameter name to indicate the parameter is intentionally unused, which follows Go best practices for unused parameters.

✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ 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 plan to trigger planning for file edits and PR creation.
  • @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
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR simplifies the HealthCheck middleware by replacing the existing NewHealthChecker function with a more concise New function and updates the corresponding configuration and documentation accordingly.

  • The middleware function has been renamed from NewHealthChecker to New.
  • The configuration type for the probe has been updated from a custom HealthChecker type to a plain function.
  • Documentation examples and references have been updated to reflect these changes.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
middleware/healthcheck/healthcheck_test.go Updated tests to use New function in place of NewHealthChecker.
middleware/healthcheck/healthcheck.go Replaced NewHealthChecker with New and updated the associated default config.
middleware/healthcheck/config.go Modified the Probe type from HealthChecker to a function signature and updated default configurations.
docs/whats_new.md Revised documentation examples to reference the new API function New.
docs/middleware/healthcheck.md Updated signature and examples to use New instead of NewHealthChecker.

@codecov
Copy link

codecov bot commented Mar 29, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.73%. Comparing base (e90fe8a) to head (8f5b5f4).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3380      +/-   ##
==========================================
+ Coverage   83.66%   83.73%   +0.06%     
==========================================
  Files         118      118              
  Lines       11716    11716              
==========================================
+ Hits         9802     9810       +8     
+ Misses       1486     1480       -6     
+ Partials      428      426       -2     
Flag Coverage Δ
unittests 83.73% <100.00%> (+0.06%) ⬆️

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.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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: 1

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between e90fe8a and a6572af.

📒 Files selected for processing (5)
  • docs/middleware/healthcheck.md (5 hunks)
  • docs/whats_new.md (1 hunks)
  • middleware/healthcheck/config.go (2 hunks)
  • middleware/healthcheck/healthcheck.go (1 hunks)
  • middleware/healthcheck/healthcheck_test.go (8 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
middleware/healthcheck/config.go (3)
middleware/logger/config.go (1)
  • Config (12-94)
app.go (1)
  • Config (135-411)
middleware/encryptcookie/config.go (1)
  • Config (8-35)
🪛 GitHub Check: lint
middleware/healthcheck/config.go

[failure] 30-30:
unused-parameter: parameter 'c' seems to be unused, consider removing or renaming it as _ (revive)

🪛 GitHub Actions: golangci-lint
middleware/healthcheck/config.go

[warning] 30-30: unused-parameter: parameter 'c' seems to be unused, consider removing or renaming it as _ (revive)

⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: repeated
  • GitHub Check: Compare
🔇 Additional comments (20)
middleware/healthcheck/healthcheck.go (1)

7-8: Function signature updated to match established conventions

The middleware now follows common Go and Fiber naming patterns by using New instead of NewHealthChecker. This simplifies the API while maintaining identical functionality.

docs/whats_new.md (4)

1567-1568: Documentation correctly reflects the new API

The examples have been properly updated to use the new healthcheck.New() function for the liveness endpoint.


1574-1574: Simplified readiness endpoint configuration

The readiness endpoint configuration is now more straightforward, using healthcheck.New() without additional parameters.


1578-1582: New startup endpoint with descriptive configuration

The documentation properly introduces the new default startup endpoint with a clear example of how to configure it with a custom probe function.


1585-1585: Simplified custom liveness endpoint example

The custom liveness endpoint example has been updated to match the new API pattern, making it consistent with other examples.

middleware/healthcheck/healthcheck_test.go (7)

37-39: Test cases updated to use new API

Test cases for default endpoints have been properly updated to use the simplified New() function.


56-58: Default test cases consistently updated

All default endpoint test cases have been properly updated to use the simplified New() function.


76-95: Custom test cases adapted to new API

The test cases for custom configurations have been properly updated to use the new API while maintaining the same test coverage and behavior verification.


126-140: Nested path test cases updated correctly

Test cases for nested path configurations have been properly migrated to the new API.


167-171: Next function test case updated

The test case for the Next configuration option has been properly updated to use the new API.


187-189: Benchmark tests updated

Benchmark tests have been properly updated to use the new API while maintaining the same performance testing criteria.


209-211: Parallel benchmark tests updated

Parallel benchmark tests have been properly updated to use the new API while maintaining the same performance testing criteria.

docs/middleware/healthcheck.md (6)

30-30: API signature updated in documentation

The function signature documentation has been updated to use the new New function name, maintaining consistency with the implementation.


48-54: Examples updated to reflect new API

All examples for different endpoint configurations have been updated to use the new healthcheck.New() function, providing clear guidance for users.


57-79: Custom configuration examples updated

Examples with custom configurations have been updated to use the new API, maintaining the same options and behavior explanations.


84-88: Application router examples updated

The example showing usage with app.All has been properly updated to use the new API.


111-111: Config type definition updated

The Probe field type has been updated from the removed HealthChecker type to its underlying function signature func(fiber.Ctx) bool, simplifying the API while maintaining the same functionality.


120-120: Default function signature improved

The default probe function signature now explicitly names the context parameter as c, improving code readability and consistency.

middleware/healthcheck/config.go (2)

21-21: LGTM: More direct function type improves API usability

Changing from a custom HealthChecker type to a function type directly is a good simplification. This makes the API more straightforward to use and aligns with Go's idiomatic approach of using function types when appropriate.


32-32: LGTM: Improved naming convention

Removing the version suffix from the function name (defaultConfigV3defaultConfig) makes the code more maintainable and less confusing for new developers. This change aligns with the PR's objective of simplifying the middleware.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.

Benchmark suite Current: 8f5b5f4 Previous: e90fe8a Ratio
Benchmark_CORS_SubdomainMatch 11.62 ns/op 0 B/op 0 allocs/op 7.699 ns/op 0 B/op 0 allocs/op 1.51
Benchmark_CORS_SubdomainMatch - ns/op 11.62 ns/op 7.699 ns/op 1.51

This comment was automatically generated by workflow using github-action-benchmark.

@gaby gaby moved this to In Progress in v3 Mar 29, 2025
@ReneWerner87 ReneWerner87 merged commit 75281bd into main Mar 30, 2025
17 of 18 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in v3 Mar 30, 2025
@gaby gaby deleted the update-healthcheck-mw branch March 30, 2025 12:14
efectn pushed a commit to ckoch786/fiber that referenced this pull request May 15, 2025
* Simplify middleware

* Rename default endpoints
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.

4 participants