Fix #8012 - Privacy plugin crashes on HTTP errors

[Lucas-C]

I tested manually this change and it works fine for my case:

$ mkdocs serve --watch-theme --open
INFO    -  Building documentation...
INFO    -  Cleaning site directory
INFO    -  Downloading external file: https://unpkg.com/mermaid@11/dist/mermaid.min.js
INFO    -  Downloading external file: https://api.star-history.com/svg?repos=py-pdf/fpdf2
ERROR   -  Could not retrieve https://api.star-history.com/svg?repos=py-pdf/fpdf2: 503 Server Error: Service Unavailable for
           url: https://api.star-history.com/svg?repos=py-pdf/fpdf2

I also tested providing an non-resolvable URL as image, and one that only answers after a long delay, and those cases are well handled now.
There is a test file you can use:

![](https://api.star-history.com/svg?repos=py-pdf/fpdf2) <!-- Triggers a HTTP 5XX -->

![](https://this-does-not-exist.com) <!-- Triggers a NameResolutionError -->

![](https://httpbin.org/delay/10) <!-- Triggers a timeout -->

I ran black on the source files, which caused several unrelated changes.

[Lucas-C]

I currently have errors like this that I need to figure how to fix:

$ mkdocs serve --watch-theme --open
INFO    -  Building documentation...
INFO    -  Cleaning site directory
INFO    -  Downloading external file: https://unpkg.com/mermaid@11/dist/mermaid.min.js
INFO    -  Downloading external file: https://this-does-not-exist.com
ERROR   -  Could not retrieve https://this-does-not-exist.com: HTTPSConnectionPool(host='this-does-not-exist.com',
           port=443): Max retries exceeded with url: / (Caused by NameResolutionError("<urllib3.connection.HTTPSConnection
           object at 0x7f9b76b093f0>: Failed to resolve 'this-does-not-exist.com' ([Errno -2] Name or service not known)"))
...
Traceback (most recent call last):
  File "~/.local/share/virtualenvs/mkdocs-material/bin/mkdocs", line 8, in <module>
    sys.exit(cli())
  File "~/.local/share/virtualenvs/mkdocs-material/lib/python3.10/site-packages/click/core.py", line 1161, in __call__
    return self.main(*args, **kwargs)
  File "~/.local/share/virtualenvs/mkdocs-material/lib/python3.10/site-packages/click/core.py", line 1082, in main
    rv = self.invoke(ctx)
  File "~/.local/share/virtualenvs/mkdocs-material/lib/python3.10/site-packages/click/core.py", line 1697, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "~/.local/share/virtualenvs/mkdocs-material/lib/python3.10/site-packages/click/core.py", line 1443, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "~/.local/share/virtualenvs/mkdocs-material/lib/python3.10/site-packages/click/core.py", line 788, in invoke
    return __callback(*args, **kwargs)
  File "~/.local/share/virtualenvs/mkdocs-material/lib/python3.10/site-packages/mkdocs/__main__.py", line 272, in serve_command
    serve.serve(**kwargs)
  File "~/.local/share/virtualenvs/mkdocs-material/lib/python3.10/site-packages/mkdocs/commands/serve.py", line 85, in serve
    builder(config)
  File "~/.local/share/virtualenvs/mkdocs-material/lib/python3.10/site-packages/mkdocs/commands/serve.py", line 67, in builder
    build(config, serve_url=None if is_clean else serve_url, dirty=is_dirty)
  File "~/.local/share/virtualenvs/mkdocs-material/lib/python3.10/site-packages/mkdocs/commands/build.py", line 347, in build
    config.plugins.on_post_build(config=config)
  File "~/.local/share/virtualenvs/mkdocs-material/lib/python3.10/site-packages/mkdocs/plugins.py", line 602, in on_post_build
    return self.run_event('post_build', config=config)
  File "~/.local/share/virtualenvs/mkdocs-material/lib/python3.10/site-packages/mkdocs/plugins.py", line 568, in run_event
    result = method(**kwargs)
  File "~/mkdocs-material/material/plugins/privacy/plugin.py", line 205, in on_post_build
    file.copy_file()
  File "~/.local/share/virtualenvs/mkdocs-material/lib/python3.10/site-packages/mkdocs/structure/files.py", line 485, in copy_file
    utils.copy_file(self.abs_src_path, output_path)
  File "~/.local/share/virtualenvs/mkdocs-material/lib/python3.10/site-packages/mkdocs/utils/__init__.py", line 123, in copy_file
    shutil.copyfile(source_path, output_path)
  File "/usr/lib/python3.10/shutil.py", line 254, in copyfile
    with open(src, 'rb') as fsrc:
FileNotFoundError: [Errno 2] No such file or directory: '/opt/mkdocs-material-bug-repro/.cache/plugin/privacy/assets/external/this-does-not-exist.com'

[Lucas-C]

I think this is now ready for reviewing, I fixed all problematic casesI could find.

However, from the logs, it seems that all failing HTTP requests are performed twice: is this normal?

[squidfunk]

Thanks for the PR! Before I review this

I ran black on the source files, which caused several unrelated changes.

Please don't do this and please stick to the current style. We'll switch to code formatting in the future (although we're likely going for ruff), but right now, this will break the conventions that we use. Nothing is worse than unfollowed conventions, although the conventions might be debatable. Once those are out, we can discuss the changes.

[squidfunk]

However, from the logs, it seems that all failing HTTP requests are performed twice: is this normal?

With the current design of the privacy plugin, yes. It first tries to fetch all resources from Markdown, which it does in parallel, and when pages are rendered, does a second pass to catch all resources from the templates. This allows to make some requests in parallel, because when the page is built, we need to wait for the downloads to complete.

This is not ideal, but a limitation of MkDocs plugin API, since we only get the hooks that there are. We could mark resources that we downloaded for "did not work", but it might be temporary, so I'd say we just leave it at that, since it's nothing that impact functionality. The build should probably fail anyway, so the author can fix it.

[Lucas-C]

I ran black on the source files, which caused several unrelated changes.

Please don't do this and please stick to the current style. [...] Once those are out, we can discuss the changes.

I reverted the formatting changes.

We could mark resources that we downloaded for "did not work", but it might be temporary, so I'd say we just leave it at that, since it's nothing that impact functionality. The build should probably fail anyway, so the author can fix it.

That's fine with me.
Note that the build will currently NOT fail, and only report errors in logs, in case of broken links.
I think that is the "good" default behaviour to have, later on a "strict" or "fail-on-broken-links" mode could be added.

The reason I think this is the "good" default behaviour is because a link will become "broken" for reasons outside the scope of a documentation source code. That means that, if we failed the build in case of broken links, for 2 consecutive "builds" of the same documentation, one could end up with a ✅ success and then just after with a ❌ failure. IMHO this is not good design, because it could lead to non-reproductible / unstable builds. But an optional toggle is fine for people who would like to be "strict" about broken links.

What do you think about this?

---

PS: as a side note / humble feedback, having the same source file duplicated twice in a source code is not really ideal / DRY. Maybe you could have a simple "copy" step during the build process for this theme, so that material/plugins is copied into src/ during the publishing step?

[squidfunk]

I think that is the "good" default behaviour to have, later on a "strict" or "fail-on-broken-links" mode could be added.

No need, just run mkdocs build --strict and it will fail. This is also why this should be a warning (see above)

PS: as a side note / humble feedback, having the same source file duplicated twice in a source code is not really ideal / DRY. Maybe you could have a simple "copy" step during the build process for this theme, so that material/plugins is copied into s rc/ during the publishing step?

Thanks for your remarks. I agree. The theme must be installable from git, but yes, it's annoying. However, the material is entirely generated, and so we must also copy the Python file. However, as I mentioned, we're currently transforming Material for MkDocs into something much more powerful, and in the process, greatly improve the overall DX.

[squidfunk]

Thanks! LGTM.