fix: logging handler must not crash its caller on reopen failure
When decnet.system.log is root-owned (e.g. created by a pre-fix 'sudo
decnet deploy') and a subsequent non-root process tries to log, the
InodeAwareRotatingFileHandler raised PermissionError out of emit(),
which propagated up through logger.debug/info and killed the collector's
log stream loop ('log stream ended ... reason=[Errno 13]').
Now matches stdlib behaviour: wrap _open() in try/except OSError and
defer to handleError() on failure. Adds a regression test.
Also: scripts/profile/view.sh 'pyinstrument' keyword was matching
memray-flamegraph-*.html files. Exclude the memray-* prefix.
This commit is contained in:
@@ -48,5 +48,13 @@ class InodeAwareRotatingFileHandler(logging.handlers.RotatingFileHandler):
|
|||||||
self.close()
|
self.close()
|
||||||
except Exception: # nosec B110
|
except Exception: # nosec B110
|
||||||
pass
|
pass
|
||||||
self.stream = self._open()
|
try:
|
||||||
|
self.stream = self._open()
|
||||||
|
except OSError:
|
||||||
|
# A logging handler MUST NOT crash its caller. If we can't
|
||||||
|
# reopen (e.g. file is root-owned after `sudo decnet deploy`
|
||||||
|
# and the current process is non-root), defer to the stdlib
|
||||||
|
# error path, which just prints a traceback to stderr.
|
||||||
|
self.handleError(record)
|
||||||
|
return
|
||||||
super().emit(record)
|
super().emit(record)
|
||||||
|
|||||||
@@ -34,7 +34,9 @@ case "${1:-}" in
|
|||||||
cprofile) TARGET="$(pick_newest '*.prof')" ;;
|
cprofile) TARGET="$(pick_newest '*.prof')" ;;
|
||||||
memray) TARGET="$(pick_newest 'memray-*.bin')" ;;
|
memray) TARGET="$(pick_newest 'memray-*.bin')" ;;
|
||||||
pyspy) TARGET="$(pick_newest 'pyspy-*.svg')" ;;
|
pyspy) TARGET="$(pick_newest 'pyspy-*.svg')" ;;
|
||||||
pyinstrument) TARGET="$(pick_newest '*.html')" ;;
|
pyinstrument) TARGET="$(find "${DIR}" -maxdepth 1 -type f -name '*.html' \
|
||||||
|
! -name 'memray-*' -printf '%T@ %p\n' 2>/dev/null \
|
||||||
|
| sort -n | tail -n 1 | cut -d' ' -f2-)" ;;
|
||||||
*) TARGET="$1" ;;
|
*) TARGET="$1" ;;
|
||||||
esac
|
esac
|
||||||
|
|
||||||
|
|||||||
@@ -78,6 +78,26 @@ def test_no_reopen_when_file_is_stable(tmp_path, monkeypatch):
|
|||||||
assert path.read_text().splitlines() == ["one", "two"]
|
assert path.read_text().splitlines() == ["one", "two"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_emit_does_not_raise_when_reopen_fails(tmp_path, monkeypatch):
|
||||||
|
"""A failed reopen must not propagate — it would crash the caller
|
||||||
|
(observed in the collector worker when decnet.system.log was root-owned
|
||||||
|
and the collector ran non-root)."""
|
||||||
|
path = tmp_path / "app.log"
|
||||||
|
h = _make_handler(path)
|
||||||
|
h.emit(_record("first"))
|
||||||
|
os.remove(path) # force reopen on next emit
|
||||||
|
|
||||||
|
def boom(*_a, **_kw):
|
||||||
|
raise PermissionError(13, "Permission denied")
|
||||||
|
monkeypatch.setattr(h, "_open", boom)
|
||||||
|
|
||||||
|
# Swallow the stderr traceback stdlib prints via handleError.
|
||||||
|
monkeypatch.setattr(h, "handleError", lambda _r: None)
|
||||||
|
|
||||||
|
# Must not raise.
|
||||||
|
h.emit(_record("second"))
|
||||||
|
|
||||||
|
|
||||||
def test_rotation_by_size_still_works(tmp_path):
|
def test_rotation_by_size_still_works(tmp_path):
|
||||||
"""maxBytes-triggered rotation must still function on top of the inode check."""
|
"""maxBytes-triggered rotation must still function on top of the inode check."""
|
||||||
path = tmp_path / "app.log"
|
path = tmp_path / "app.log"
|
||||||
|
|||||||
Reference in New Issue
Block a user