Skip to content

Allow errors.py as the error handler module's name (error.py also works)#47

Merged
volfpeter merged 3 commits into
mainfrom
feat/allow-errors.py-as-error-handler-module-name
Jan 3, 2026
Merged

Allow errors.py as the error handler module's name (error.py also works)#47
volfpeter merged 3 commits into
mainfrom
feat/allow-errors.py-as-error-handler-module-name

Conversation

@volfpeter
Copy link
Copy Markdown
Owner

@volfpeter volfpeter commented Jan 3, 2026

Closes #46

Summary by CodeRabbit

  • Documentation
    • Error handling docs expanded: handlers may reside in either error.py or errors.py; guidance added on rendered-error behavior, redirects, and recommending a reusable HTML skeleton for consistent styling.
    • README/docs: new Testimonials section and minor typographic tweaks.
  • Chores
    • Package version bumped to 0.8.0.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jan 3, 2026

📝 Walkthrough

Walkthrough

Support for root-module names "error.py" and "errors.py" was added across docs, types, and loader (preferring errors.py when both exist); logging was centralized to a module-level logger and the package version was bumped.

Changes

Cohort / File(s) Summary
Documentation
docs/application-components.md, docs/guides/error-handling.md, docs/index.md, README.md
Docs updated to accept either error.py or errors.py as the root error-handler module and minor copy/format tweaks (new Testimonials section, emphasis change).
Type definitions
holm/typing.py
RootOnlyModuleName literal expanded from Literal["error"] to Literal["error","errors"], enlarging recognized root module names.
Error handler loading
holm/modules/_error.py
Added load_error_handler_owner(pkg: PackageInfo) which tries errors then error, logs a warning if both exist, and returns the chosen owner or None.
App integration
holm/app.py
Replaced use of is_error_handler_owner/direct import with load_error_handler_owner(pkg) when registering error handlers.
Logging centralization
holm/logging.py, holm/_model.py
Introduced module-level logger = getLogger("holm") and switched _model.py to use the centralized logger rather than creating a local logger.
Version bump
holm/__init__.py
Package version updated from 0.7.4 to 0.8.0.

Sequence Diagram(s)

(Skipped — changes are localized and do not introduce a multi-component sequential flow requiring visualization.)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 Hopping through modules, both names in my paw,
I find errors or error—I'll use what I saw.
A single logger hums, one version raised on high,
Error pages chosen with a curious sigh.
Nibble, patch, and bounce — code carrots in the sky!

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning Several changes appear out of scope: testimonials added to README and docs, version bump to 0.8.0, and centralized logger creation unrelated to the error handler module naming feature. Remove testimonials from README.md and docs/index.md, consider reverting version bump to 0.7.5, and discuss whether the logger refactoring in _model.py and logging.py belongs in this feature branch.
✅ 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 clearly and specifically describes the main change: allowing errors.py as an alternative error handler module name while maintaining error.py support.
Linked Issues check ✅ Passed The PR implements all requirements from issue #46: accepting both error.py and errors.py as error handler module names, with load_error_handler_owner() trying both variants and prioritizing errors.py.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6a3f4d1 and 1249055.

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

[style] ~29-~29: Consider using a more formal and expressive alternative to ‘awesome’.
Context: ... the holm/htmy/FastHX stack, it's awesome." (David Brochart, Technical Director a...

(AWESOME)

docs/index.md

[style] ~20-~20: Consider using a more formal and expressive alternative to ‘awesome’.
Context: ... the holm/htmy/FastHX stack, it's awesome." (David Brochart, Technical Director a...

(AWESOME)

🔇 Additional comments (4)
README.md (2)

27-30: Testimonial quote appears authentic but uses informal language.

The testimonial quote uses informal language ("awesome") which static analysis flags. Since this is presented as a direct quote from David Brochart (with a GitHub issue reference), it's appropriate to preserve the original wording. However, verify that the quote is verbatim from the linked GitHub issue to ensure accuracy of attribution.


33-33: Formatting change is idiomatic.

The switch from asterisks to underscores for emphasis (_basic_ instead of *basic*) is idiomatic Markdown and aligns with consistency across the documentation.

docs/index.md (2)

18-21: Testimonials section placement and content are appropriate.

The testimonials section is well-positioned after the feature list and before prerequisites, providing social proof to users early in their documentation journey. The quote and attribution are consistent with the README.md entry. Verify that the quote is verbatim from the referenced GitHub issue.


24-24: Formatting change maintains consistency across documentation.

The underscore-based emphasis (_basic_) matches the corresponding change in README.md and is idiomatic Markdown.


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
Copy Markdown

@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/guides/error-handling.md (1)

11-11: Document the precedence when both files exist.

The documentation mentions that both error.py and errors.py are supported but doesn't clarify what happens when both files are present in the same directory. Based on the AI summary, errors.py takes precedence, but this should be explicitly documented to avoid user confusion.

Suggested documentation update

Consider adding a note like:

-With all that said, let's set up error handling by adding the `error.py` or `errors.py` file to the root of our project (next to `main.py`).
+With all that said, let's set up error handling by adding the `error.py` or `errors.py` file to the root of our project (next to `main.py`). If both files exist, `errors.py` takes precedence.
docs/application-components.md (1)

113-113: Document the file precedence behavior.

Similar to the guide, this documentation should clarify what happens when both error.py and errors.py exist. This is particularly important in this comprehensive documentation section.

Suggested clarification

Consider adding a note about precedence:

-Error handlers are defined in the `error.py` or `errors.py` module of the root package as a `handlers` variable, which can be a mapping from exception types or HTTP status codes to standard FastAPI exception handler functions, or a callable that expects a `fasthx.htmy.HTMY` positional argument and returns such a mapping. The latter option is useful if you want to do custom HTML rendering in an error handler.
+Error handlers are defined in the `error.py` or `errors.py` module of the root package as a `handlers` variable, which can be a mapping from exception types or HTTP status codes to standard FastAPI exception handler functions, or a callable that expects a `fasthx.htmy.HTMY` positional argument and returns such a mapping. If both files exist, `errors.py` takes precedence. The latter option is useful if you want to do custom HTML rendering in an error handler.
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1ea515c and 36a715b.

📒 Files selected for processing (8)
  • docs/application-components.md
  • docs/guides/error-handling.md
  • holm/_model.py
  • holm/app.py
  • holm/logging.py
  • holm/modules/_error.py
  • holm/typing.py
  • test_app/errors.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-22T23:11:22.422Z
Learnt from: volfpeter
Repo: volfpeter/holm PR: 42
File: examples/forms/page.py:15-15
Timestamp: 2025-12-22T23:11:22.422Z
Learning: In htmy v0.10.0 and later, do not use html tag objects (e.g., html.div, html.form, html.head) as type hints or return types. Replace these with htmy's ComponentType or Component for type annotations in Python code (e.g., def render(...) -> Component or def handler(...) -> ComponentType). Apply this consistently across Python files where htmy HTML tag objects are used in type hints.

Applied to files:

  • holm/logging.py
  • holm/modules/_error.py
  • holm/_model.py
  • holm/app.py
  • holm/typing.py
🧬 Code graph analysis (2)
holm/modules/_error.py (1)
holm/_model.py (2)
  • PackageInfo (103-190)
  • import_module (124-159)
holm/app.py (2)
holm/modules/_error.py (2)
  • load_error_handler_owner (34-50)
  • register_error_handlers (53-68)
holm/modules/_layout.py (1)
  • htmy (78-79)
🔇 Additional comments (8)
holm/logging.py (1)

1-3: LGTM!

The centralized logger setup follows Python logging best practices and provides a consistent logging interface for the entire package.

holm/typing.py (1)

6-6: LGTM!

The type expansion correctly adds "errors" as a valid root-only module name while maintaining backward compatibility with existing "error" usage. The change automatically propagates through ModuleName and the module_names set.

holm/_model.py (2)

10-10: LGTM!

The refactor to use the centralized logger from holm.logging improves consistency across the package while maintaining identical logging behavior.


152-153: LGTM!

The logger usage remains functionally identical after the centralization refactor.

holm/modules/_error.py (2)

8-10: LGTM! Import additions support the new function.

The added imports (PackageInfo and logger) are necessary for the new load_error_handler_owner function and are correctly sourced from internal modules.


34-50: LGTM! Clean implementation that achieves the PR objective.

The function correctly attempts to load both errors.py and error.py, prioritizing errors.py when both exist. The warning message is helpful for users who might have accidentally created both modules. The implementation handles all edge cases appropriately (neither exists, one exists, both exist).

holm/app.py (2)

22-22: LGTM! Import addition is correct.

The addition of load_error_handler_owner to the imports is necessary for the updated error handler loading approach on line 55.


54-55: LGTM! Clean refactoring that centralizes error handler loading.

The updated approach correctly delegates error handler module discovery to load_error_handler_owner, which now supports both error.py and errors.py naming conventions. The refactoring improves code organization by centralizing this logic in the _error module.

@volfpeter volfpeter merged commit f9fc45b into main Jan 3, 2026
3 checks passed
@volfpeter volfpeter deleted the feat/allow-errors.py-as-error-handler-module-name branch January 3, 2026 21:52
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.

Allow errors.py as the error handler module's name

1 participant