ReadTheDocs autodetection can be a small footgun for non-RTD builds / make_tag raises exceptions when `ogp_site_url` is None

Julian commented 1 year ago

(Hello! Thanks for the library!)

I've recently began to notice CI dirhtml builds which use sphinxext.opengraph fail after bumping dependencies. The kinds of errors (across projects) look like:

⊙  tox -e docs-dirhtml -s                                                                                                         julian@Airm ●
docs-dirhtml installed: alabaster==0.7.12,astroid==2.12.13,attrs==22.1.0,Babel==2.11.0,beautifulsoup4==4.11.1,certifi==2022.9.24,charset-normalizer==2.1.1,docutils==0.19,furo==2022.9.29,idna==3.4,imagesize==1.4.1,jedi==0.18.2,Jinja2==3.1.2,jsonschema @ file:///.,lazy-object-proxy==1.8.0,lxml==4.9.1,MarkupSafe==2.1.1,packaging==21.3,parso==0.8.3,pudb==2022.1.3,pyenchant==3.2.2,Pygments==2.13.0,pyparsing==3.0.9,pyrsistent==0.19.2,pytz==2022.6,PyYAML==6.0,requests==2.28.1,snowballstemmer==2.2.0,soupsieve==2.3.2.post1,Sphinx==5.3.0,sphinx-autoapi==2.0.0,sphinx-basic-ng==1.0.0b1,sphinx-copybutton==0.5.1,sphinx_autodoc_typehints==1.19.5,sphinx_json_schema_spec==2.3.1,sphinxcontrib-applehelp==1.0.2,sphinxcontrib-devhelp==1.0.2,sphinxcontrib-htmlhelp==2.0.0,sphinxcontrib-jsmath==1.0.1,sphinxcontrib-qthelp==1.0.3,sphinxcontrib-serializinghtml==1.1.5,sphinxcontrib-spelling==7.7.0,sphinxext-opengraph==0.7.2,Unidecode==1.3.6,urllib3==1.26.12,urwid==2.1.2,urwid-readline==0.13,wrapt==1.14.1
docs-dirhtml run-test-pre: PYTHONHASHSEED='911188001'
docs-dirhtml run-test: commands[0] | /Users/julian/Development/jsonschema/.tox/docs-dirhtml/bin/python -m sphinx -b dirhtml /Users/julian/Development/jsonschema/docs/ /Users/julian/Development/jsonschema/.tox/docs-dirhtml/tmp/build -a -n -q -T -W

Traceback (most recent call last):
  File "/Users/julian/Development/jsonschema/.tox/docs-dirhtml/lib/python3.11/site-packages/sphinx/events.py", line 94, in emit
    results.append(listener.handler(self.app, *args))
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/julian/Development/jsonschema/.tox/docs-dirhtml/lib/python3.11/site-packages/sphinxext/opengraph/__init__.py", line 192, in html_page_context
    context["metatags"] += get_tags(app, context, doctree, app.config)
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/julian/Development/jsonschema/.tox/docs-dirhtml/lib/python3.11/site-packages/sphinxext/opengraph/__init__.py", line 176, in get_tags
    [make_tag(p, c) for p, c in tags.items()]
  File "/Users/julian/Development/jsonschema/.tox/docs-dirhtml/lib/python3.11/site-packages/sphinxext/opengraph/__init__.py", line 176, in <listcomp>
    [make_tag(p, c) for p, c in tags.items()]
     ^^^^^^^^^^^^^^
  File "/Users/julian/Development/jsonschema/.tox/docs-dirhtml/lib/python3.11/site-packages/sphinxext/opengraph/__init__.py", line 34, in make_tag
    content = content.replace('"', "&quot;")
              ^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'replace'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/Users/julian/Development/jsonschema/.tox/docs-dirhtml/lib/python3.11/site-packages/sphinx/cmd/build.py", line 281, in build_main
    app.build(args.force_all, args.filenames)
  File "/Users/julian/Development/jsonschema/.tox/docs-dirhtml/lib/python3.11/site-packages/sphinx/application.py", line 343, in build
    self.builder.build_all()
  File "/Users/julian/Development/jsonschema/.tox/docs-dirhtml/lib/python3.11/site-packages/sphinx/builders/__init__.py", line 269, in build_all
    self.build(None, summary=__('all source files'), method='all')
  File "/Users/julian/Development/jsonschema/.tox/docs-dirhtml/lib/python3.11/site-packages/sphinx/builders/__init__.py", line 376, in build
    self.write(docnames, list(updated_docnames), method)
  File "/Users/julian/Development/jsonschema/.tox/docs-dirhtml/lib/python3.11/site-packages/sphinx/builders/__init__.py", line 571, in write
    self._write_serial(sorted(docnames))
  File "/Users/julian/Development/jsonschema/.tox/docs-dirhtml/lib/python3.11/site-packages/sphinx/builders/__init__.py", line 581, in _write_serial
    self.write_doc(docname, doctree)
  File "/Users/julian/Development/jsonschema/.tox/docs-dirhtml/lib/python3.11/site-packages/sphinx/builders/html/__init__.py", line 672, in write_doc
    self.handle_page(docname, ctx, event_arg=doctree)
  File "/Users/julian/Development/jsonschema/.tox/docs-dirhtml/lib/python3.11/site-packages/sphinx/builders/html/__init__.py", line 1074, in handle_page
    newtmpl = self.app.emit_firstresult('html-page-context', pagename,
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/julian/Development/jsonschema/.tox/docs-dirhtml/lib/python3.11/site-packages/sphinx/application.py", line 475, in emit_firstresult
    return self.events.emit_firstresult(event, *args,
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/julian/Development/jsonschema/.tox/docs-dirhtml/lib/python3.11/site-packages/sphinx/events.py", line 115, in emit_firstresult
    for result in self.emit(name, *args, allowed_exceptions=allowed_exceptions):
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/julian/Development/jsonschema/.tox/docs-dirhtml/lib/python3.11/site-packages/sphinx/events.py", line 105, in emit
    raise ExtensionError(__("Handler %r for event %r threw an exception") %
sphinx.errors.ExtensionError: Handler <function html_page_context at 0x10a976de0> for event 'html-page-context' threw an exception (exception: 'NoneType' object has no attribute 'replace')

Extension error (sphinxext.opengraph):
Handler <function html_page_context at 0x10a976de0> for event 'html-page-context' threw an exception (exception: 'NoneType' object has no attribute 'replace')
ERROR: InvocationError for command /Users/julian/Development/jsonschema/.tox/docs-dirhtml/bin/python -m sphinx -b dirhtml /Users/julian/Development/jsonschema/docs/ /Users/julian/Development/jsonschema/.tox/docs-dirhtml/tmp/build -a -n -q -T -W (exited with code 2)
___________________________________________________________________ summary ____________________________________________________________________
ERROR:   docs-dirhtml: commands failed

where there's replace being called on None rather than a str.

Some minor debugging showed that what's happening is that autodetection of the READTHEDOCS environment variable obviously doesn't work when the build is running in CI (or in tox locally), so ogp_site_url ends up being None, which make_tag won't like very much.

I didn't track down what changed if anything (either here or in other deps) -- I'm happy to do so if that's helpful, as I've definitely successfully used this library before, but it seems that things are a bit awkward if one relies entirely on the ReadTheDocs autodetection, so I thought I'd file this anyhow. Specifically I of course want my site URL autodetected, and "real" doc builds will run in RTD. But it seems without manually specifying ogp_site_url, I can't actually test doc builds.

Concrete suggestions (possibly not all needed) in case that's helpful (and I'm sure I may be missing even better ones):

mention in the documentation that relying on ReadTheDocs autodetection may mean you cannot run dirhtml externally
handle None in make_tag or in the frame above it (and do some nonsense thing, since that may be better than jjust blowing up)
provide guidance for how to set ogp_site_url just for this case (CI for builds) in a way that the value isn't "in danger" of being used, but where it's still usable during CI builds (e.g. via an envvar which is used only when READTHEDOCS is not set or something)

(Again I may have missed something, so apologies for any errors! And thanks again)

TheTripleV commented 1 year ago

Sorry, there's an open PR for this issue I've forgotten about. I'll see to that.

Julian commented 1 year ago

No need at all for apologies! I saw that PR and wasn't 100% sure it was meant to address this specifically, but yeah thanks for the response and having a look!

Julian commented 1 year ago

Oh! I lied! I saw #56 and completely missed #84 which of course is relevant!

TheTripleV commented 1 year ago

v0.7.3 has been released and should fix your issue.

Julian commented 1 year ago

Amazing, thank you! Indeed seems to.

wpilibsuite / sphinxext-opengraph

ReadTheDocs autodetection can be a small footgun for non-RTD builds / make_tag raises exceptions when `ogp_site_url` is None #87