Skip to content

Conversation

@ysmoradi
Copy link
Member

@ysmoradi ysmoradi commented Sep 24, 2025

closes #11433

Summary by CodeRabbit

  • Bug Fixes

    • Improved handling of 401/403/404 responses for more consistent and predictable error pages.
    • Eliminated occasional fall-through behavior on certain error responses.
  • Refactor

    • Streamlined error handling setup to enhance maintainability without changing user-facing behavior.
    • Unified application base path configuration across environments for more consistent runtime behavior.

@ysmoradi ysmoradi requested a review from Copilot September 24, 2025 14:12
@coderabbitai
Copy link

coderabbitai bot commented Sep 24, 2025

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Refactors 40X status code handling by converting a private middleware configurator into a WebApplication extension method and updating its call site. Adjusts the status code pipeline to avoid invoking Next for specific scenarios. Separately, sets the builder ContentRootPath to AppContext.BaseDirectory unconditionally.

Changes

Cohort / File(s) Summary
40X middleware refactor
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Web/Program.Middlewares.cs
Replaced Configure_401_403_404_Pages(app) with app.Handle40XStatusCodes(); turned method into a WebApplication extension; removed the else branch that called statusCodeContext.Next, altering 40X handling flow.
Builder configuration
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Web/Program.cs
Removed preprocessor conditional; always sets ContentRootPath = AppContext.BaseDirectory in WebApplication.CreateBuilder options.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Client
  participant WA as WebApplication
  participant SCP as StatusCodePages Middleware
  participant H as Handle40XStatusCodes (custom)

  C->>WA: HTTP request
  WA->>SCP: Pass through pipeline
  alt 401/403/404
    SCP->>H: Invoke custom handler
    note right of H: New behavior short-circuits<br/>without calling Next
    H-->>C: Return 40X response
  else Other status / pass-through
    SCP-->>WA: Continue pipeline
    WA-->>C: Response
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I tap my paws on status code doors—tap, tap, tap!
401, 403, 404—no longer a maze or trap.
An extension hop, a cleaner stop,
Content roots aligned—no flip, no flop.
Carrots deployed; the pipeline’s tight—hip-hop! 🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Out of Scope Changes Check ⚠️ Warning The PR also unconditionally changes ContentRootPath assignment in Program.cs (removing the conditional preprocessor around ContentRootPath = AppContext.BaseDirectory), which is unrelated to improving 401/403/404 status-code handling and therefore appears out of scope for the linked issue. Other edits in the middleware files are on-topic, but the Program.cs change should be justified or split out. Move the ContentRootPath change into a separate PR or include a justification in this PR description explaining why the ContentRootPath change is required for the 401/403/404 handling changes; if unrelated, revert it from this branch to keep the PR focused.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title concisely and accurately describes the main intent of the changeset—improving 401, 403 and 404 status code handling—and matches the middleware refactor and status-code handling updates in the diff. It is a single clear sentence and readable for reviewers. The appended issue number is extraneous but does not make the title misleading.
Linked Issues Check ✅ Passed The submitted changes directly modify the 40X handling middleware by converting Configure_401_403_404_Pages to an app.Handle40XStatusCodes extension and updating the pipeline registration, and the removal of the previous StatusCodePages else branch changes how Next is invoked; these code changes map to the linked issue objective to improve 401/403/404 handling. There are no other coding requirements listed in the linked issue, and the diffs show the intended refactor that targets those status-code behaviors. No automated tests or additional behavioral documentation are present in the summary, so reviewers should verify runtime behavior, but the implementation aligns with the stated objective.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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

Comment @coderabbitai help to get the list of available commands and usage tips.

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 improves the handling of 401, 403, and 404 status codes in the bit Boilerplate template by removing unnecessary conditional comments and refactoring the status code handling logic.

  • Removes conditional compilation comments from WebApplication configuration
  • Renames the status code handling method for better clarity
  • Simplifies the middleware logic by removing an unnecessary else block

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
Program.cs Removes conditional compilation comments around ContentRootPath setting
Program.Middlewares.cs Renames status code handler method and simplifies middleware logic

Copy link

@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)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Web/Program.Middlewares.cs (1)

183-198: Consider performance optimization for path checking.

The middleware performs string operations on every request, including those that don't need status code handling. Consider caching the path check results or moving these checks inside the StatusCodePages handler for better performance.

-        app.Use(async (context, next) =>
-        {
-            if (context.Request.Path.HasValue)
-            {
-                if (context.Request.Path.Value.Contains(PageUrls.NotFound, StringComparison.InvariantCultureIgnoreCase))
-                {
-                    context.Response.StatusCode = (int)HttpStatusCode.NotFound;
-                }
-                if (context.Request.Path.Value.Contains(PageUrls.NotAuthorized, StringComparison.InvariantCultureIgnoreCase))
-                {
-                    context.Response.StatusCode = context.Request.Query["isForbidden"].FirstOrDefault() is "true" ? (int)HttpStatusCode.Forbidden : (int)HttpStatusCode.Unauthorized;
-                }
-            }
-
-            await next.Invoke(context);
-        });
+        app.Use(async (context, next) =>
+        {
+            var path = context.Request.Path.Value;
+            if (!string.IsNullOrEmpty(path))
+            {
+                if (path.Contains(PageUrls.NotFound, StringComparison.InvariantCultureIgnoreCase))
+                {
+                    context.Response.StatusCode = (int)HttpStatusCode.NotFound;
+                }
+                else if (path.Contains(PageUrls.NotAuthorized, StringComparison.InvariantCultureIgnoreCase))
+                {
+                    context.Response.StatusCode = context.Request.Query["isForbidden"].FirstOrDefault() is "true" ? (int)HttpStatusCode.Forbidden : (int)HttpStatusCode.Unauthorized;
+                }
+            }
+
+            await next.Invoke(context);
+        });
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 4d01c9b and 5160d5b.

📒 Files selected for processing (2)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Web/Program.Middlewares.cs (2 hunks)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Web/Program.cs (0 hunks)
💤 Files with no reviewable changes (1)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Web/Program.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build and test
🔇 Additional comments (4)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Web/Program.Middlewares.cs (4)

57-57: LGTM: Clean API improvement with extension method pattern.

The change from direct method call to extension method call improves the API design by making the middleware registration feel more natural in the pipeline chain.


181-181: Good refactor to extension method pattern.

Converting the private method to an extension method improves the API design and makes it more consistent with other middleware registration patterns in ASP.NET Core.


206-220: No action required — redirects are scoped to Blazor components or unmatched requests.

401/403 redirect only runs when the endpoint's metadata contains ComponentTypeMetadata (Blazor) and 404 redirect only when GetEndpoint() is null, so mapped API controllers (api/... via MapControllers) are not affected.


200-222: Verified — no else/Next removed; other status codes unaffected.

Inspected src/Templates/Boilerplate/.../Program.Middlewares.cs and other UseStatusCodePages instances: HandleAsync only redirects 401/403 (Blazor endpoints) and 404 (no endpoint); there is no removed else or Next() invocation — other status codes continue to flow to the normal pipeline.

@ysmoradi ysmoradi merged commit b929fd8 into bitfoundation:develop Sep 25, 2025
3 checks passed
@ysmoradi ysmoradi deleted the 11433 branch September 25, 2025 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

401, 403 and 404 status codes handling in bit Boilerplate must get improved

1 participant