Skip to content

fix: record lost a mark born during the block, and the backtrace of what threw - #25

Merged
sotashimozono merged 1 commit into
mainfrom
fix/record-loses-newborn-probes-and-backtraces
Sep 7, 2026
Merged

fix: record lost a mark born during the block, and the backtrace of what threw#25
sotashimozono merged 1 commit into
mainfrom
fix/record-loses-newborn-probes-and-backtraces

Conversation

@sotashimozono

Copy link
Copy Markdown
Member

Two defects in record, both surfaced by reviewing #24 and both older than it. The first breaks
the one thing the default layer promises.

A mark born while the block ran was lost — from the always-on layer too

record snapshotted the probe set before calling f and never looked again. A probe that came
into existence during the call was entered by code that ran, counted by nobody, and left with its
flag false for the rest of the process — because while a recording is open the write side
counts into the probe instead of setting the flag, and only record's epilogue sets it, over the
stale snapshot.

before:  record saw [:tracked]              entered(D) = [:tracked]
after:   record saw [:newborn, :tracked]    entered(D) = [:newborn, :tracked]

newborn had run and returned a value in both cases.

A package extension loaded inside the block is the ordinary way this happens, and this package
ships three of them. So entered() and the exit summary — the layer whose whole claim is that it
cannot miss an entry — would have said nothing.

The probe set is now re-derived after the call, saved is keyed by probe rather than by position,
and reconciliation runs over the union. Re-deriving needs invokelatest: the new probes' bindings
are younger than the frame reading them, and 1.12 warns that will become an error.

The exception's backtrace pointed at record, not at the caller

throw(err) after the catch block manufactures a fresh backtrace:

via record, before:  ["error", "record", "top-level scope", …]
via record, after:   ["error", "energy", "deep", "mid", "outer", …]
direct call:         ["error", "energy", "deep", "mid", "outer", …]

A user debugging a failed run saw record.jl where their own call chain should be, with nothing
to say frames had been dropped — in exactly the case the docstring names as the reason to reach
for rethrow = false. Closing now happens inside the catch and the exception is re-raised with
rethrow(), which keeps the backtrace it arrived with.

The comment that claimed this was impossible was right about Base.rethrow(err) outside a catch
and wrong about the conclusion: the call had simply drifted to after the catch.

Tests

Both fail against the unfixed file, and each carries a control — a mark defined and never called
is still absent (so the fix did not start reporting everything it can see), and the direct call is
shown to carry the frames the recorded one must also carry.

1062 assertions, green.

🤖 Generated with Claude Code

…hat threw

Two defects in `record`, both found by reviewing #24 and both older than it. The first breaks the
one thing the default layer promises.

**A mark that came into existence WHILE the block ran was lost — from the always-on layer too.**
`record` snapshotted the probe set before calling `f` and never looked again, so a probe born
during the call was entered by code that ran, counted by nobody, and left with its flag `false`
for the rest of the process. `entered()` and the exit summary never learned about it either,
because while a recording is open the write side counts into the probe instead of setting the
flag, and only `record`'s epilogue sets it — over the stale snapshot.

    record saw: [:tracked]                 entered(D) = [:tracked]
    …after:     [:newborn, :tracked]       entered(D) = [:newborn, :tracked]

A package extension loaded inside the block is the ordinary way this happens, and this package
ships three of them. The probe set is now re-derived after the call, `saved` is keyed by probe
rather than by position, and the reconciliation runs over the union. Re-deriving needs
`invokelatest`: the new probes' bindings are younger than the frame reading them, and 1.12 warns
that will become an error.

**The exception's backtrace pointed at `record`, not at the caller.** `throw(err)` after the
`catch` block manufactures a fresh backtrace, so a user debugging a failed run saw `record.jl` and
macro expansion where `outer → mid → deep → energy` should be, with nothing to say frames had been
dropped — in exactly the case `record(f; rethrow = false)`'s own docstring names as the reason to
use it. Closing now happens inside the `catch` and the exception is re-raised with `rethrow()`,
which keeps the backtrace it arrived with. The old comment claiming this was impossible was wrong:
it is impossible *after* the catch, which is where the call had drifted to.

Both are pinned by tests that fail against the unfixed file, each with a control — a mark defined
and never called is still absent, and the direct call is shown to carry the frames the recorded
one must also carry.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions github-actions Bot added the bug Something isn't working label Sep 7, 2026
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

📚 Docs preview: https://codes.sota-shimozono.com/ExperimentalAPI.jl/previews/PR25/

(updates on each push to this PR)

@codecov

codecov Bot commented Sep 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@sotashimozono
sotashimozono merged commit 071a9f4 into main Sep 7, 2026
14 checks passed
@sotashimozono
sotashimozono deleted the fix/record-loses-newborn-probes-and-backtraces branch September 7, 2026 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant