-
Type:
Task
-
Resolution: Unresolved
-
Priority:
Major - P3
-
None
-
Affects Version/s: None
-
Component/s: None
-
None
-
Server Programmability
-
None
-
3
-
TBD
-
None
-
None
-
None
-
None
-
None
-
None
We should pull in libassert [1] and use it to back invariant (which automatically handles dassert) and possibly tassert in developer local builds and unit test tasks in CI. The latter are special, and more like local builds than other CI builds because they compile and run the tests with debug symbols and source locally available rather than shipping stripped binaries to be tested in a separate task. This will result in significantly more human friendly output on failure. However, because it does not use structured logging and captures the inputs to the assertion without redaction, we must ensure it is never used in production builds.
Note that this likely depends on (or should be done together with) SERVER-94540 since it uses cpptrace for backtraces, and if we are pulling that in, we might as well use it too.
A few bits of configuration were I think we will want to differ from the defaults (leaving to implementor to make final call though):
- set_path_mode(full) - by default it will try to shorten the paths. While it does look better that way, I think it will interfere with vscode autolinking/cmd-clickability, so we should just use whole paths. They aren't that long.
- We should make the stringifier work well for our code. The simplest option may be to make libassert::stringifier go through mongo::unittest::stringify::invoke(), and move the later out of the unittest namespace. At the very least, we will probably need to do something about boost::optional and its broken SFINAE-unfriendly operator<<.
- Register a handler to write the output to the log file (which is stdout by default) while holding the lock. This must bypass the structured logging and write directly to the file, otherwise it defeats the purpose of making this more human readable. It may be reasonable to defer this for follow up work.
- We should consider making separate calls to assertion.header() and assertion.get_stacktrace().print_with_snippets() (or using a configured cpptrace::formatter) rather than using the all-in-one assertion.to_string() in order to get "snippets", printing the relevant source code + N lines of context directly.
- Enable LIBASSERT_USE_FMT
- Either enable LIBASSERT_DECOMPOSE_BINARY_LOGICAL after validating that it does the right things for !p || *p == 1 (not dereferencing null) and p && *p == 1 (failing the assert before dereferencing null) or write a clang-tidy rule to automatically add (), disabling decomposition for invariants that include && or ||. The latter is probably only required if it doesn't compile without it.
As pointed out in the slack discussion, we will need some way to effectively unittest our production invariant() code, even if we are using this in unittest builds.
[1] While this ticket is currently phrased in terms of libassert specifically, we may want to choose a different library as part of this work.
- depends on
-
SERVER-94540 Improve C++ backtrace usability during local development
-
- Open
-