sphinx-needs icon indicating copy to clipboard operation
sphinx-needs copied to clipboard

Missing variant value for external link causes an error

Open David-Le-Nir opened this issue 2 years ago • 2 comments

When using variant on a field that defines a link. When this field has no default value and a "unknown" tag is used there is an error.

For example, I have a field "link" that defines a remote link. For a need, I want to define this field only for one variant.

.. need:: test
   :link: var1:EXT-NEED-001

When I build the document for variant var2, I have the following error:

Traceback (most recent call last):
  File "/usr/local/lib/python3.9/dist-packages/sphinx/events.py", line 94, in emit
    results.append(listener.handler(self.app, *args))
  File "/usr/local/lib/python3.9/dist-packages/sphinx_needs/directives/need.py", line 368, in process_need_nodes
    check_links(env)
  File "/usr/local/lib/python3.9/dist-packages/sphinx_needs/directives/need.py", line 443, in check_links
    for link in need_link_value:
TypeError: 'NoneType' object is not iterable
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
  File "/usr/local/lib/python3.9/dist-packages/sphinx/cmd/build.py", line 277, in build_main
    app.build(args.force_all, filenames)
  File "/usr/local/lib/python3.9/dist-packages/sphinx/application.py", line 343, in build
    self.builder.build_all()
  File "/usr/local/lib/python3.9/dist-packages/sphinx/builders/__init__.py", line 265, in build_all
    self.build(None, summary=__('all source files'), method='all')
  File "/usr/local/lib/python3.9/dist-packages/sphinx/builders/__init__.py", line 367, in build
    self.write(docnames, list(updated_docnames), method)
  File "/usr/local/lib/python3.9/dist-packages/sphinx/builders/__init__.py", line 559, in write
    self._write_serial(sorted(docnames))
  File "/usr/local/lib/python3.9/dist-packages/sphinx/builders/__init__.py", line 566, in _write_serial
    doctree = self.env.get_and_resolve_doctree(docname, self)
  File "/usr/local/lib/python3.9/dist-packages/sphinx/environment/__init__.py", line 530, in get_and_resolve_doctree
    self.apply_post_transforms(doctree, docname)
  File "/usr/local/lib/python3.9/dist-packages/sphinx/environment/__init__.py", line 581, in apply_post_transforms
    self.events.emit('doctree-resolved', doctree, docname)
  File "/usr/local/lib/python3.9/dist-packages/sphinx/events.py", line 105, in emit
    raise ExtensionError(__("Handler %r for event %r threw an exception") %
sphinx.errors.ExtensionError: Handler <function process_need_nodes at 0x7f805a84eb80> for event 'doctree-resolved' threw an exception (exception: 'NoneType' object is not iterable)
Extension error (sphinx_needs.directives.need):
Handler <function process_need_nodes at 0x7f805a84eb80> for event 'doctree-resolved' threw an exception (exception: 'NoneType' object is not iterable)

I would have expected that there is no error, and that the field would not have been displayed.

I also tried to set:

.. need:: test
   :link: var1:EXT-NEED-001, ''

Which avoid the error but displays the field with '' value, reported as an invalid link.

David-Le-Nir avatar May 30 '23 09:05 David-Le-Nir

Good finding. I think having options for one variant only is not supported. But for sure, it should be.

But this means also, later code must check on its own if a value is empty/None. This is normally done by Sphinx itself (no empty options) and most later code trusted this check. Will not be easy to identify all of this places or to find a generic way to recheck it in advance.

danwos avatar May 30 '23 11:05 danwos

I understand your point for the later code.

My 2 cents: I am checking some values in my needs to raise functional/semantic warnings if required. Prior to use the variant in the documents, the checks where quite simple. Introducing variants increased, sometimes a lot, the code complexity (value defined or not for the parameterised tag(s), empty and default values, ...).

For some of these cases (e.g. #915, ) I had to use dynamic function in place of variant. For empty values on links it seems to work. There is no error has the dynamic function returns an empty array by default if missing the variant value. The field is not displayed when not defined. So maybe the later code should not be too impacted by a simple fix such as forcing a default empty list when getting links

e.g something like this proposal for the line 443 could be enough

need_link_value = (
                [need[link_type["option"]]] if isinstance(need[link_type["option"]], str) else need[link_type["option"]]
            ) or []

Variant is a super helpful feature for my users and I would greatly prefer to use the variant instead of my non-user-friendly workaround.

David-Le-Nir avatar May 30 '23 15:05 David-Le-Nir