Preserve both the body failure and release failure from lock contexts

[mmashwani]

Severity: Enhancement (when the protected operation and release both fail, callers receive only the release failure as the propagated exception)

Summary

I would like the sync and async context-manager APIs to provide an explicit
dual-failure contract. If the protected body raises and release() also
raises, the release exception becomes the propagated exception. The body
exception remains only in __context__.

Release failures are important because ownership may remain live, but the
protected-operation failure is usually the primary application error. Both
need a stable, documented representation that logging and error handlers can
process without inspecting implicit exception chaining.

Environment

• package/version: filelock 3.29.7
• source: commit 1efb8932c08789deb08ad93a91d8996cc36bb9cc
• reproduced platform: macOS, local APFS filesystem
• affected APIs: sync and async lock context managers

Reproduction

1. Run the attached filelock_context_release_masks_body.py with
   filelock==3.29.7.

2. Observe:

   filelock_version=3.29.7
   sync_direct.propagated_exception=OSError
   sync_direct.body_exception_in_context=ValueError
   sync_proxy.propagated_exception=OSError
   sync_proxy.body_exception_in_context=ValueError
   async_direct.propagated_exception=OSError
   async_direct.body_exception_in_context=ValueError
   async_proxy.propagated_exception=OSError
   async_proxy.body_exception_in_context=ValueError
   

Observed vs expected

Observed:

The cleanup failure replaces the body failure as the exception propagated by
the with statement. The second failure is represented only by implicit
chaining.

Expected:

The public contract preserves both failures explicitly and consistently for
sync and async contexts.

Root cause (verified in source, 3.29.7)

BaseFileLock.__exit__()
and
AcquireReturnProxy.__exit__()
call release() without using the body exception arguments.

BaseAsyncFileLock.__aexit__()
and
AsyncAcquireReturnProxy.__aexit__()
have the same shape with await release().

This is normal Python behavior, not a Python runtime bug. The Python language
reference specifies that with passes the body exception to __exit__(), and
that async with awaits __aexit__(*sys.exc_info()) on the exception path:
https://docs.python.org/3/reference/compound_stmts.html#the-with-statement
and
https://docs.python.org/3/reference/compound_stmts.html#the-async-with-statement.
If that exit method raises, the release exception becomes the newly propagated
exception and the body exception is left to implicit exception chaining.

Suggested fix

Define and document a dual-failure policy. Suitable designs include a
filelock-specific aggregate exception that exposes body_exception and
release_exception, or ExceptionGroup where the supported Python runtime
provides it plus an equivalent structured form on older supported runtimes.

The policy should retain the complete original traceback for both failures and
must not suppress the release failure, because that failure can mean ownership
was not relinquished.

Verification

• Body succeeds, release fails: the release failure remains visible.
• Body fails, release succeeds: the original body exception remains unchanged.
• Body and release fail: both exceptions and tracebacks are available through
  the documented structure.
• Direct lock contexts and acquire-return proxies behave identically.
• Sync and async APIs use the same policy.

I did not find an existing upstream issue covering this exact dual-failure
contract. A targeted search found PR #219, but that was about thread-local
locking behavior and is not a duplicate.

Current workaround

Avoid the context-manager shorthand when both failures must be handled
explicitly. Acquire first, run the body under try, and perform release in a
separate error-aggregation block.

Reproduction script

filelock_context_release_masks_body.py

"""Show which exception escapes when a lock body and release both fail."""

from __future__ import annotations

import asyncio
import tempfile
from pathlib import Path
from unittest.mock import patch

import filelock
from filelock import AsyncFileLock
from filelock import FileLock


def exception_name(exc: BaseException | None) -> str | None:
    return type(exc).__name__ if exc is not None else None


def body_context_name(exc: BaseException | None) -> str | None:
    context = exc.__context__ if exc is not None else None
    return exception_name(context)


def sync_direct(path: Path) -> BaseException | None:
    lock = FileLock(path, timeout=0)
    caught: BaseException | None = None

    with patch.object(type(lock), "_release", side_effect=OSError("injected release failure")):
        try:
            with lock:
                raise ValueError("sync direct body failure")
        except BaseException as exc:  # capture the exception that callers receive
            caught = exc

    if lock.is_locked:
        lock.release(force=True)
    return caught


def sync_proxy(path: Path) -> BaseException | None:
    lock = FileLock(path, timeout=0)
    caught: BaseException | None = None

    with patch.object(type(lock), "_release", side_effect=OSError("injected release failure")):
        try:
            with lock.acquire():
                raise ValueError("sync acquire proxy body failure")
        except BaseException as exc:  # capture the exception that callers receive
            caught = exc

    if lock.is_locked:
        lock.release(force=True)
    return caught


async def async_direct(path: Path) -> BaseException | None:
    lock = AsyncFileLock(path, timeout=0)
    caught: BaseException | None = None

    with patch.object(type(lock), "_release", side_effect=OSError("injected release failure")):
        try:
            async with lock:
                raise ValueError("async direct body failure")
        except BaseException as exc:  # capture the exception that callers receive
            caught = exc

    if lock.is_locked:
        await lock.release(force=True)
    return caught


async def async_proxy(path: Path) -> BaseException | None:
    lock = AsyncFileLock(path, timeout=0)
    caught: BaseException | None = None

    with patch.object(type(lock), "_release", side_effect=OSError("injected release failure")):
        try:
            manager = await lock.acquire()
            async with manager:
                raise ValueError("async acquire proxy body failure")
        except BaseException as exc:  # capture the exception that callers receive
            caught = exc

    if lock.is_locked:
        await lock.release(force=True)
    return caught


def main() -> int:
    with tempfile.TemporaryDirectory() as directory:
        base_path = Path(directory)
        results = {
            "sync_direct": sync_direct(base_path / "sync-direct.lock"),
            "sync_proxy": sync_proxy(base_path / "sync-proxy.lock"),
            "async_direct": asyncio.run(async_direct(base_path / "async-direct.lock")),
            "async_proxy": asyncio.run(async_proxy(base_path / "async-proxy.lock")),
        }

        print(f"filelock_version={filelock.__version__}")
        for name, caught in results.items():
            print(f"{name}.propagated_exception={exception_name(caught)}")
            print(f"{name}.body_exception_in_context={body_context_name(caught)}")

        reproduced = all(
            isinstance(caught, OSError) and isinstance(caught.__context__, ValueError)
            for caught in results.values()
        )

        return 0 if reproduced else 1


if __name__ == "__main__":
    raise SystemExit(main())

[gaborbernat]

CPython standard-library inclusion needs a separate proposal, maintainer support, API-stability commitments, and a fit with stdlib maintenance constraints. No change in this repository can deliver that outcome, so closing.

[gaborbernat]

Reopening — I closed this against a handoff whose issue-number mapping did not match this issue's actual content, so the reason I posted was wrong. Re-reviewing against the real issue text and will follow up correctly. Apologies for the noise.