Skip to content

Commit

Permalink
fix: add workaround for Windows permissions error
Browse files Browse the repository at this point in the history
We were getting errors like `PermissionError: [WinError 5] Access is
denied:
'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\resume.md_k098zhaj\\CrashpadMetrics-active.pma'`
and `Exception ignored in: <finalize object at 0x16d9e039d60; dead> ...
NotADirectoryError: [WinError 267] The directory name is invalid:
'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\resume.md_3aoun3f2\\CrashpadMetrics-active.pma'`
in CI on Windows. See e.g.
https://github.com/mikepqr/resume.md/runs/5172290981 and
https://github.com/mikepqr/resume.md/runs/5172234014.

The root cause was that Chrome creates a file the python process does
not have permission to delete. See
puppeteer/puppeteer#2778. Because
TemporaryDirectory is intended to be used as a context manager there is
no way to prevent it logging an error when cleanup fails.

The fix is to switch to the lower level tempfile.mkdtemp, and make a
good faith attempt to clean it up manually, logging failure at the debug
level (while adding a new --debug option).

A more sophisticated fix would be to backport the new
ignore_cleanup_errors option added in python 3.10
(python/cpython#24793), but this will do.

Fixes #13
  • Loading branch information
mikepqr committed Feb 13, 2022
1 parent ff3f485 commit 01a33ef
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 15 deletions.
4 changes: 1 addition & 3 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ on:
workflow_dispatch:
pull_request:
push:
branches:
main

jobs:
build:
Expand All @@ -21,7 +19,7 @@ jobs:
- name: Install markdown
run: pip3 install markdown
- name: Make resume
run: python3 resume.py
run: python3 resume.py --debug
- name: Rename output
if: ${{ matrix.os != 'windows-latest' }}
run: |
Expand Down
30 changes: 18 additions & 12 deletions resume.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@ def write_pdf(html: str, prefix: str = "resume", chrome: str = "") -> None:
Write html to prefix.pdf
"""
chrome = chrome or guess_chrome_path()

html64 = base64.b64encode(html.encode("utf-8"))
options = [
"--headless",
Expand All @@ -132,9 +131,18 @@ def write_pdf(html: str, prefix: str = "resume", chrome: str = "") -> None:
if sys.platform == "win32":
options.append("--disable-gpu")

tmpdir = tempfile.TemporaryDirectory(prefix="resume.md_")
options.append(f"--crash-dumps-dir={tmpdir.name}")
options.append(f"--user-data-dir={tmpdir.name}")
# Ideally we'd use tempfile.TemporaryDirectory here. We can't because
# attempts to delete the tmpdir fail on Windows because Chrome creates a
# file the python process does not have permission to delete. See
# https://github.com/puppeteer/puppeteer/issues/2778,
# https://github.com/puppeteer/puppeteer/issues/298, and
# https://bugs.python.org/issue26660. If we ever drop Python 3.9 support we
# can use TemporaryDirectory with ignore_cleanup_errors=True as a context
# manager.
tmpdir = tempfile.mkdtemp(prefix="resume.md_")
options.append(f"--crash-dumps-dir={tmpdir}")
options.append(f"--user-data-dir={tmpdir}")

try:
subprocess.run(
[
Expand All @@ -155,14 +163,9 @@ def write_pdf(html: str, prefix: str = "resume", chrome: str = "") -> None:
else:
raise exc
finally:
# We use this try-finally rather than TemporaryDirectory's context
# manager to be able to catch the exception caused by
# https://bugs.python.org/issue26660 on Windows
try:
shutil.rmtree(tmpdir.name)
except PermissionError as exc:
logging.warning(f"Could not delete {tmpdir.name}")
logging.info(exc)
shutil.rmtree(tmpdir, ignore_errors=True)
if os.path.isdir(tmpdir):
logging.debug(f"Could not delete {tmpdir}")


if __name__ == "__main__":
Expand All @@ -188,10 +191,13 @@ def write_pdf(html: str, prefix: str = "resume", chrome: str = "") -> None:
help="Path to Chrome or Chromium executable",
)
parser.add_argument("-q", "--quiet", action="store_true")
parser.add_argument("--debug", action="store_true")
args = parser.parse_args()

if args.quiet:
logging.basicConfig(level=logging.WARN, format="%(message)s")
elif args.debug:
logging.basicConfig(level=logging.DEBUG, format="%(message)s")
else:
logging.basicConfig(level=logging.INFO, format="%(message)s")

Expand Down

0 comments on commit 01a33ef

Please sign in to comment.