Skip to content

馃悰 Warn on needservice naming an unregistered service instead of crashing - #2103

Closed
havinash-007 wants to merge 2 commits into
useblocks:masterfrom
havinash-007:fix/needservice-unknown-service
Closed

havinash-007 wants to merge 2 commits into
useblocks:masterfrom
havinash-007:fix/needservice-unknown-service

Conversation

@havinash-007

Copy link
Copy Markdown

Closes #2101

What

A needservice naming an unregistered service ended the build with a raw traceback: ServiceManager.get raises NeedsServiceException, which derived from BaseException and was not caught in NeedserviceDirective.run, so it also bypassed Sphinx's own except Exception handling.

  • NeedserviceDirective.run now catches it and reports a located needs.directive warning (same text: the service name and the registered ones), then returns no nodes so the build continues (-W fails it like any other warning).
  • NeedsServiceException now derives from Exception (as suggested in the issue; nothing in the repo relies on it bypassing except Exception).
  • Regression test tests/test_service_unknown.py (fails before with the exception, passes after) and a changelog entry.

Checks

  • Tests added (test_service_unknown.py; fails before the change, passes after)
  • Changelog entry in docs/changelog.rst
  • uv run poe lint (all prek hooks, incl. ruff and ty) passes
  • uv run pytest packages/sphinx-needs/tests -k service passes
  • Full sphinx-needs suite: same result as before the change apart from the new passing test. 121 failures and 46 errors occur identically on a clean checkout (all PlantUML rendering, no renderer in my environment)
  • Docs: no user-facing documentation page needed changing

AI assistance

This change was prepared with Claude Code (AI agent); I reviewed the diff and test results.

馃 Generated with Claude Code

NeedsServiceException derived from BaseException and was not caught in
NeedserviceDirective.run, ending the build with a raw traceback. Report a
needs.directive warning and return no nodes, and derive the exception from
Exception.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added the pkg: sphinx-needs The sphinx-needs distribution (packages/sphinx-needs): its code, tests and docs label Oct 7, 2026
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@chrisjsewell

Copy link
Copy Markdown
Member

Thanks for the quick fix. #2101 is now fixed through #2113, so I'm closing this one in its favour.

#2113 fixes it together with #2067: the registration check there means a service whose class cannot be registered is skipped with a warning, and a needservice naming it, or naming a service that never existed, takes the same path, a located needs.load_service_need warning with the existing not-found text, with NeedsServiceException an Exception as here. Appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pkg: sphinx-needs The sphinx-needs distribution (packages/sphinx-needs): its code, tests and docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

馃悰 needservice naming an unregistered service ends the build with a raw BaseException traceback instead of a warning

2 participants