polish: add `#[track_caller]` to functions that can panic #4413

carllerche · 2022-01-20T20:54:46Z

Our current MSRV is 1.46, which introduced track_caller which can be used to make panic messages better.

We should ensure that all methods that can panic when misused are annotated with #[track_caller] so the panic message includes the file/line where the user called the Tokio method instead of showing the file/line inside the Tokio source. If the panic is a few calls deep in Tokio, then I believe each internal method needs its own #[track_caller]

One example where this should be done: Handle::current(), but there probably are others.

Ideally, we also add tests that verify the behavior.

The text was updated successfully, but these errors were encountered:

Functions that may panic can be annotated with `#[track_caller]` so that in the event of a panic, the function where the user called the panicking function is shown instead of the file and line within Tokio source. This change adds track caller to all the non-unstable public APIs in Tokio core where the documentation describes how the function may panic due to incorrect context or inputs. Since each internal function needs to be annotated down to the actual panic, it makes sense to start in Tokio core functionality. Tests are needed to ensure that all the annotations remain in place in case internal refactoring occurs. The test installs a panic hook to extract the file location from the `PanicInfo` struct and clone it up to the outer scope to check that the panic was indeed reported from within the test file. The downside to this approach is that the panic hook is global while set and so we need a lot of extra functionality to effectively serialize the tests so that only a single panic can occur at a time. The annotation of `block_on` was removed as it did not work. It appears to be impossible to correctly chain track caller when the call stack to the panic passes through clojures, as the track caller annotation can only be applied to functions. Also, the panic message itself is very descriptive.

Functions that may panic can be annotated with `#[track_caller]` so that in the event of a panic, the function where the user called the panicking function is shown instead of the file and line within Tokio source. This change adds `#[track_caller]` to all the non-unstable public APIs in tokio-util where the documentation describes how the function may panic due to incorrect context or inputs. Tests are included to cover each potentially panicking function.

Functions that may panic can be annotated with `#[track_caller]` so that in the event of a panic, the function where the user called the panicking function is shown instead of the file and line within Tokio source. This change adds `#[track_caller]` to all the non-unstable public APIs in tokio-util where the documentation describes how the function may panic due to incorrect context or inputs. In one place, an assert was added where the described behavior appeared not to be implemented. The documentation for `DelayQueue::reserve` states that the function will panic if the new capacity exceeds the maximum number of entries the queue can contain. However, the function didn't panic until a higher number caused by an allocation failure. This is inconsistent with `DelayQueue::insert_at` which will panic if the number of entries were to go over MAX_ENTRIES. Tests are included to cover each potentially panicking function.

Functions that may panic can be annotated with `#[track_caller]` so that in the event of a panic, the function where the user called the panicking function is shown instead of the file and line within Tokio source. This change adds `#[track_caller]` to all the non-unstable public APIs in tokio-stream (only `chunks_timeout` in `StreamExt`) where the documentation describes how this function may panic due to incorrect input. A test has been included to cover the panic.

Functions that may panic can be annotated with `#[track_caller]` so that in the event of a panic, the function where the user called the panicking function is shown instead of the file and line within Tokio source. This change adds track caller to all the non-unstable public APIs in Tokio core where the documentation describes how the function may panic due to incorrect context or inputs. Since each internal function needs to be annotated down to the actual panic, it makes sense to start in Tokio core functionality. Tests are needed to ensure that all the annotations remain in place in case internal refactoring occurs. The test installs a panic hook to extract the file location from the `PanicInfo` struct and clone it up to the outer scope to check that the panic was indeed reported from within the test file. The downside to this approach is that the panic hook is global while set and so we need a lot of extra functionality to effectively serialize the tests so that only a single panic can occur at a time. The annotation of `block_on` was removed as it did not work. It appears to be impossible to correctly chain track caller when the call stack to the panic passes through clojures, as the track caller annotation can only be applied to functions. Also, the panic message itself is very descriptive.

Functions that may panic can be annotated with `#[track_caller]` so that in the event of a panic, the function where the user called the panicking function is shown instead of the file and line within Tokio source. This change adds `#[track_caller]` to all the non-unstable public APIs in tokio-util where the documentation describes how the function may panic due to incorrect context or inputs. In one place, an assert was added where the described behavior appeared not to be implemented. The documentation for `DelayQueue::reserve` states that the function will panic if the new capacity exceeds the maximum number of entries the queue can contain. However, the function didn't panic until a higher number caused by an allocation failure. This is inconsistent with `DelayQueue::insert_at` which will panic if the number of entries were to go over MAX_ENTRIES. Tests are included to cover each potentially panicking function. Refs: tokio-rs#4413

Functions that may panic can be annotated with `#[track_caller]` so that in the event of a panic, the function where the user called the panicking function is shown instead of the file and line within Tokio source. This change adds `#[track_caller]` to all the non-unstable public APIs in tokio-stream (only `chunks_timeout` in `StreamExt`) where the documentation describes how this function may panic due to incorrect input. A test has been included to cover the panic.

Functions that may panic can be annotated with `#[track_caller]` so that in the event of a panic, the function where the user called the panicking function is shown instead of the file and line within Tokio source. This change adds `#[track_caller]` to all the non-unstable public APIs in tokio-stream (only `chunks_timeout` in `StreamExt`) where the documentation describes how this function may panic due to incorrect input. A test has been included to cover the panic. Refs: tokio-rs#4413

Functions that may panic can be annotated with `#[track_caller]` so that in the event of a panic, the function where the user called the panicking function is shown instead of the file and line within Tokio source. This change adds `#[track_caller]` to all the non-unstable public APIs in tokio-stream (only `chunks_timeout` in `StreamExt`) where the documentation describes how this function may panic due to incorrect input. A test has been included to cover the panic. Refs: #4413

Functions that may panic can be annotated with #[track_caller] so that in the event of a panic, the function where the user called the panicking function is shown instead of the file and line within Tokio source. This change adds #[track_caller] to all the non-unstable public APIs in tokio-util where the documentation describes how the function may panic due to incorrect context or inputs. In cases where `#[track_caller]` does not work, it has been left out. For example, it currently does not work on async functions, blocks, or closures. So any call stack that passes through one of these before reaching the actual panic is not able to show the calling site outside of tokio as the panic location. The public functions that could not have `#[track_caller]` added for this reason are: * `time::advance` Tests are included to cover each potentially panicking function. In the following cases, `#[track_caller]` had already been added, and only tests have been added: * `time::interval` * `time::interval_at` Refs: tokio-rs#4413

* util: add track_caller to public APIs Functions that may panic can be annotated with `#[track_caller]` so that in the event of a panic, the function where the user called the panicking function is shown instead of the file and line within Tokio source. This change adds `#[track_caller]` to all the non-unstable public APIs in tokio-util where the documentation describes how the function may panic due to incorrect context or inputs. In one place, an assert was added where the described behavior appeared not to be implemented. The documentation for `DelayQueue::reserve` states that the function will panic if the new capacity exceeds the maximum number of entries the queue can contain. However, the function didn't panic until a higher number caused by an allocation failure. This is inconsistent with `DelayQueue::insert_at` which will panic if the number of entries were to go over MAX_ENTRIES. Tests are included to cover each potentially panicking function. Refs: #4413 * fix tests on FreeBSD 32-bit (I hope) Some tests were failing on FreeBSD 32-bit because the "times too far in the future" for DelayQueue were also too far in the future for the OS. Fixed by copying the MAX_DURATION value from where it's defined and using it to create a duration that is just 1 more than the maximum. This will start to break once we get close (within 2 and a bit years) of the Epochalypse (19 Jan, 2038) - but a lot of other things are going to be breaking on FreeBSD 32-bit by then anyway.

Functions that may panic can be annotated with #[track_caller] so that in the event of a panic, the function where the user called the panicking function is shown instead of the file and line within Tokio source. This change adds #[track_caller] to all the non-unstable public APIs in tokio-util where the documentation describes how the function may panic due to incorrect context or inputs. In cases where `#[track_caller]` does not work, it has been left out. For example, it currently does not work on async functions, blocks, or closures. So any call stack that passes through one of these before reaching the actual panic is not able to show the calling site outside of tokio as the panic location. The public functions that could not have `#[track_caller]` added for this reason are: * `time::advance` Tests are included to cover each potentially panicking function. In the following cases, `#[track_caller]` had already been added, and only tests have been added: * `time::interval` * `time::interval_at` Refs: #4413

Functions that may panic can be annotated with #[track_caller] so that in the event of a panic, the function where the user called the panicking function is shown instead of the file and line within Tokio source. This change adds #[track_caller] to all the non-unstable public io APIs in the main tokio crate where the documentation describes how the function may panic due to incorrect context or inputs. Additionally, the documentation for `AsyncFd` was updated to indicate that the functions `new` and `with_intent` can panic. Tests are included to cover each potentially panicking function. The logic to test the location of a panic (which is a little complex), has been moved to a test support module. Refs: tokio-rs#4413

Functions that may panic can be annotated with #[track_caller] so that in the event of a panic, the function where the user called the panicking function is shown instead of the file and line within Tokio source. This change adds #[track_caller] to all the non-unstable public net APIs in the main tokio crate where the documentation describes how the function may panic due to incorrect context or inputs. Not all panic cases can have #[track_caller] applied fully as the callstack passes through a closure which isn't yet supported by the annotation (e.g. net functions called from outside a tokio runtime). Additionally, the documentation was updated to indicate additional cases in which the public net functions may panic (the same as the io functions). Tests are included to cover each potentially panicking function. Refs: tokio-rs#4413

Functions that may panic can be annotated with #[track_caller] so that in the event of a panic, the function where the user called the panicking function is shown instead of the file and line within Tokio source. This change adds #[track_caller] to the signal() function which is the only function in the signal public API which can panic. Documentation was added to this function to indicate that it may panic. Not all panic cases can have #[track_caller] applied fully as the callstack passes through a closure which isn't yet supported by the annotation (e.g. signal() called from outside a tokio runtime). Tests are included to cover the case where signal() is called from a runtime without IO enabled. Refs: tokio-rs#4413

Functions that may panic can be annotated with `#[track_caller]` so that in the event of a panic, the function where the user called the panicking function is shown instead of the file and line within Tokio source. This change adds `#[track_caller]` to all the public APIs in the sync module of the tokio crate where the documentation describes how the function may panic due to incorrect context or inputs. In cases where `#[track_caller]` does not work, it has been left out. For example, it currently does not work on async functions, blocks, or closures. So any call stack that passes through one of these before reaching the actual panic is not able to show the calling site outside of tokio as the panic location. The following functions have call stacks that pass through closures: * `sync::watch::Sender::send_modify` * `sync::watch::Sender::send_if_modified` Additionally, in the above functions it is a panic inside the supplied closure which causes the function to panic, and so showing the location of the panic itself is desirable. The following functions are async: * `sync::mpsc::bounded::Sender::send_timeout` Tests are included to cover each potentially panicking function. Refs: tokio-rs#4413

Functions that may panic can be annotated with #[track_caller] so that in the event of a panic, the function where the user called the panicking function is shown instead of the file and line within Tokio source. This change adds #[track_caller] to the signal() function which is the only function in the signal public API which can panic. Documentation was added to this function to indicate that it may panic. Not all panic cases can have #[track_caller] applied fully as the callstack passes through a closure which isn't yet supported by the annotation (e.g. signal() called from outside a tokio runtime). Tests are included to cover the case where signal() is called from a runtime without IO enabled. Refs: #4413

Functions that may panic can be annotated with `#[track_caller]` so that in the event of a panic, the function where the user called the panicking function is shown instead of the file and line within Tokio source. This change adds `#[track_caller]` to all the public APIs in the sync module of the tokio crate where the documentation describes how the function may panic due to incorrect context or inputs. In cases where `#[track_caller]` does not work, it has been left out. For example, it currently does not work on async functions, blocks, or closures. So any call stack that passes through one of these before reaching the actual panic is not able to show the calling site outside of tokio as the panic location. The following functions have call stacks that pass through closures: * `sync::watch::Sender::send_modify` * `sync::watch::Sender::send_if_modified` Additionally, in the above functions it is a panic inside the supplied closure which causes the function to panic, and so showing the location of the panic itself is desirable. The following functions are async: * `sync::mpsc::bounded::Sender::send_timeout` Tests are included to cover each potentially panicking function. Refs: #4413

Functions that may panic can be annotated with #[track_caller] so that in the event of a panic, the function where the user called the panicking function is shown instead of the file and line within Tokio source. This change adds #[track_caller] to all the non-unstable public net APIs in the main tokio crate where the documentation describes how the function may panic due to incorrect context or inputs. Not all panic cases can have #[track_caller] applied fully as the callstack passes through a closure which isn't yet supported by the annotation (e.g. net functions called from outside a tokio runtime). Additionally, the documentation was updated to indicate additional cases in which the public net functions may panic (the same as the io functions). Tests are included to cover each potentially panicking function. Refs: #4413

Functions that may panic can be annotated with `#[track_caller]` so that in the event of a panic, the function where the user called the panicking function is shown instead of the file and line within Tokio source. This change adds `#[track_caller]` to all the public APIs in the task module of the tokio crate where the documentation describes how the function may panic due to incorrect context or inputs. In cases where `#[track_caller]` does not work, it has been left out. For example, it currently does not work on async functions, blocks, or closures. So any call stack that passes through one of these before reaching the actual panic is not able to show the calling site outside of tokio as the panic location. The following functions have call stacks that pass through closures: * `task::block_in_place` * `task::local::spawn_local` Tests are included to cover each potentially panicking function. The following functions already had `#[track_caller]` applied everywhere it was needed and only tests have been added: * `task::spawn` * `task::LocalKey::sync_scope` Refs: tokio-rs#4413

Functions that may panic can be annotated with `#[track_caller]` so that in the event of a panic, the function where the user called the panicking function is shown instead of the file and line within Tokio source. This change adds `#[track_caller]` to all the public APIs in the task module of the tokio crate where the documentation describes how the function may panic due to incorrect context or inputs. In cases where `#[track_caller]` does not work, it has been left out. For example, it currently does not work on async functions, blocks, or closures. So any call stack that passes through one of these before reaching the actual panic is not able to show the calling site outside of tokio as the panic location. The following functions have call stacks that pass through closures: * `task::block_in_place` * `task::local::spawn_local` Tests are included to cover each potentially panicking function. The following functions already had `#[track_caller]` applied everywhere it was needed and only tests have been added: * `task::spawn` * `task::LocalKey::sync_scope` Refs: #4413

hds · 2022-07-21T12:44:09Z

With the merging of that last PR, I think that this issue could be considered complete. The #[track_caller] attribute has been added to all public, stable APIs where possible.

I searched the tokio repo for # Panics, i.e. places where the documentation indicates that a panic may occur when a function is used incorrectly. I then went through them all and added the attribute and tests. Functions were excluded when the call stack passed through either a closure, block, or async function as #[track_caller] cannot be applied to those. Each PR details the functions which could not be covered.

Here's a breakdown of the crates and modules covered:

Crate / module	APIs covered / total	PRs	Work left
tokio/io	10/10	#4793	✅
tokio/macros	macros don't need `#[track_caller]`
tokio/net	8/8	#4805	✅
tokio/park	Panic headings only in `Park` trait
tokio/runtime	7/7	#4772	✅
tokio/signal	3/3	#4806	✅
tokio/sync	10/13	#4808	1 x async, 2 x not needed
tokio/task	13/15	#4848	2 x closures
tokio/time	9/10	#4791	??
tokio-stream	1/1	#4786	✅
tokio-util	13/14	#4785	??
Total	74/81

Note: Unstable APIs were excluded.

Details of cases that don't work:

tokio::sync::mpsc::bounded::Sender::send_timeout - passes through an async
tokio::sync::watch::Sender::send_modify - This shouldn't have #[track_caller] on it because it only panics if the closure passed to it panics (so you want the panic in the closure to show up).
tokio::sync::watch::Sender::send_if_modified - This shouldn't have #[track_caller] on it because it only panics if the closure passed to it panics (so you want the panic in the closure to show up).

Issues (in the Rust compiler) for cases where #[track_caller] will not work:

Closures: Tracking Issue for #[track_caller] on closures rust-lang/rust#87417
Async functions: #[track_caller] does nothing on async fns rust-lang/rust#78840

CC @Darksonn (since you approved most of the PRs in this set and have been generally amazing with your quick feedback and help - Thank you!)

Darksonn · 2022-07-21T13:44:24Z

How many functions were you unable to apply #[track_caller] to? I don't really have a good overview of the count.

hds · 2022-07-21T14:54:34Z

I updated the comment above with counts of the functions covered of the total. The counts don't exactly match the search count in a given directory because it's not including unstable APIs and one case where a panic section which describes a panic that doesn't originate in that function.

This change adds a case that was missing from the original PR, tokio-rs#4793. The `io::driver::Handle::current` function was only covered by `#[track_caller]` in the case that the `rt` feature is enabled, however it was missing in the case that the `rt` feture isn't enabled (in which case a panic would be more common). Refs: tokio-rs#4413

This change adds a case that was missing from the original PR, tokio-rs#4793. The `io::driver::Handle::current` function was only covered by `#[track_caller]` in the case that the `rt` feature is enabled, however it was missing in the case that the `rt` feture isn't enabled (in which case a panic would be more common). This particular case cannot be tested in the tokio tests as they always run with all features enabled. Refs: tokio-rs#4413

hds · 2022-07-21T15:05:07Z

And while doing the counts, I realized that I missed one case where the rt feature is disabled, I've just created a small PR to add this: #4852.

This change adds a case that was missing from the original PR, #4793. The `io::driver::Handle::current` function was only covered by `#[track_caller]` in the case that the `rt` feature is enabled, however it was missing in the case that the `rt` feture isn't enabled (in which case a panic would be more common). This particular case cannot be tested in the tokio tests as they always run with all features enabled. Refs: #4413

Functions that may panic can be annotated with `#[track_caller]` so that in the event of a panic, the function where the user called the panicking function is shown instead of the file and line within Tokio source. This change adds `#[track_caller]` to two public APIs in tokio task module which weren't added in #4848. * `tokio::task::block_in_place` * `tokio::task::spawn_local` These APIs had call stacks that went through closures, which is a use case not supported by `#[track_caller]`. These two cases have been refactored so that the `panic!` call is no longer inside a closure. Tests have been added for these two new cases. Refs: #4413

…s#5034) Functions that may panic can be annotated with `#[track_caller]` so that in the event of a panic, the function where the user called the panicking function is shown instead of the file and line within Tokio source. This change adds `#[track_caller]` to two public APIs in tokio task module which weren't added in tokio-rs#4848. * `tokio::task::block_in_place` * `tokio::task::spawn_local` These APIs had call stacks that went through closures, which is a use case not supported by `#[track_caller]`. These two cases have been refactored so that the `panic!` call is no longer inside a closure. Tests have been added for these two new cases. Refs: tokio-rs#4413

carllerche · 2022-10-27T21:29:12Z

Closing as there are no more immediate steps to take. Follow up issues can be opened for more targetted work if needed.

carllerche added A-tokio Area: The main tokio crate C-feature-request Category: A feature request. E-easy Call for participation: Experience needed to fix: Easy / not much labels Jan 20, 2022

carllerche changed the title ~~chore: add #[track_caller] to functions that can panic~~ polish: add #[track_caller] to functions that can panic Jan 20, 2022

Noah-Kennedy self-assigned this Jan 20, 2022

Noah-Kennedy mentioned this issue Jan 29, 2022

track caller in tokio-util #4445

Closed

hds mentioned this issue Jun 16, 2022

add track_caller to public APIs (#4413) #4772

Merged

hds mentioned this issue Jun 22, 2022

util: add track_caller to public APIs #4785

Merged

hds mentioned this issue Jun 22, 2022

stream: add track_caller to public APIs #4786

Merged

hds mentioned this issue Jun 27, 2022

time: add track_caller to public APIs #4791

Merged

hds mentioned this issue Jul 4, 2022

net: add track_caller to public APIs #4805

Merged

hds mentioned this issue Jul 5, 2022

signal: add track_caller to public APIs #4806

Merged

hds mentioned this issue Jul 5, 2022

sync: add track_caller to public APIs #4808

Merged

hds mentioned this issue Jul 20, 2022

task: add track_caller to public APIs #4848

Merged

hds mentioned this issue Jul 21, 2022

io: add track_caller caller to no-rt io driver #4852

Merged

hds mentioned this issue Sep 20, 2022

task: add track_caller to block_in_place and spawn_local #5034

Merged

hds mentioned this issue Sep 21, 2022

Ensure tracing gets the correct spawning location for all spawn methods #5038

Open

carllerche closed this as completed Oct 27, 2022

Darksonn removed the E-easy Call for participation: Experience needed to fix: Easy / not much label Oct 29, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

polish: add `#[track_caller]` to functions that can panic #4413

polish: add `#[track_caller]` to functions that can panic #4413

carllerche commented Jan 20, 2022 •

edited

Loading

hds commented Jul 21, 2022 •

edited

Loading

Darksonn commented Jul 21, 2022

hds commented Jul 21, 2022

hds commented Jul 21, 2022

carllerche commented Oct 27, 2022

polish: add #[track_caller] to functions that can panic #4413

polish: add #[track_caller] to functions that can panic #4413

Comments

carllerche commented Jan 20, 2022 • edited Loading

hds commented Jul 21, 2022 • edited Loading

Darksonn commented Jul 21, 2022

hds commented Jul 21, 2022

hds commented Jul 21, 2022

carllerche commented Oct 27, 2022

polish: add `#[track_caller]` to functions that can panic #4413

polish: add `#[track_caller]` to functions that can panic #4413

carllerche commented Jan 20, 2022 •

edited

Loading

hds commented Jul 21, 2022 •

edited

Loading