-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Proposal: Add a way for a test to expect a panic #1356
Comments
This proposal would have the added benefit of allowing zig to self-host its runtime safety tests: https://github.com/ziglang/zig/blob/master/test/runtime_safety.zig |
For tests which call Do not add any save-me-from-the-crash feature available to the user, that's the road to hell. There's use case when you do handle very, very rare situation, but add an assert there, just to make sure that this rare situation doesn't occur by some mistake in your regular code. For this special variant of assert should be invented, as proposed in #1304 (comment). I use not very helpful name Example:
|
Please provide only one way to return and cache error.
Crash process and catch by the parent process should be a good idea. |
In my experience panic / recover is not really abused in Go much. |
@binary132, look at how the Go JSON parser cleverly uses recover to handle invalid input errors. Edit: Speak of the devil... an article on this topic: https://eli.thegreenplace.net/2018/on-the-uses-and-misuses-of-panics-in-go. |
isn't this possible in userspace using setjmp (or equivalent) to effectively create a restore point and then longjmp out of your panic handler? Pretty sure this would never fly at comptime though. |
This relates more to the LLVM IR and how it implements panics. There is definitely a mechanism in the IR to throw and recover from exceptions, but Zig might not want to expose this in the language to avoid abuse. Update: More details here: https://llvm.org/docs/ExceptionHandling.html. |
I think it's fairly common practice in Go to use Without a way to do so, any logging instrumentation added to an executable will not be able to record everything and in some environments it can be a problem if you can't do remote logging properly. An alternative could be to use an external process to do the remote logging (so that it can also read the Zig executable's final message), but it's a nuisance, especially when deploying to a container. If there are no bad technical implications, I would recommend leaving the option of using some kind of |
Hi Loris, this is not documented yet (See #1517) but you can override the default panic function in the root source file. In the panic function you define what happens when an assertion fails. The default behavior is here: https://github.com/ziglang/zig/blob/56e07622c692f70eb10836b86c5fda02c53e2394/std/special/panic.zig If you specify a panic handler in the root source file, like this: const builtin = @import("builtin");
pub fn panic(msg: []const u8, error_return_trace: ?*builtin.StackTrace) noreturn {
// your implementation here
} Here you can do this logging instrumentation and then call the default panic handler, or do some other strategy. |
Oh I see, so, if understand correctly, it can already be done but only at top level, where recover can instead be called at any level. Could there be a legitimate reason for a library to want to hide panics? Thank you for the clarification :) |
It would be rare for a library to handle panics or to expect panics to be handled. such a library would probably advertise itself as a library specifically for managing application state, such as a logging library that "owns" stdout, stderr, and the panic handler. in such a scenario, the library would document that any application that wishes to take advantage of the panic handling code should define a top-level panic handler that calls into the library's implementation. The reason there's no library-local support for handling panic is that crash-related situations have app-wide impact, and managing that is the responsibility of the app. |
What about zig DLLs loaded from C or something else? It's not uncommon in game code for example to have an executable that loads other subsystems (e.g. renderer) or the core game itself from DLLs. If the executable itself was written in C and one of those DLLs was written in zig, what would happen if a zig panic occurs? I'm thinking of a scenario where parts of a larger existing system would be rewritten one by one because porting the whole thing might not be feasible. Or maybe source code is not available for porting. |
Zig's panic system is set up to cross object file boundaries in this way, but I don't think this use case is solved yet. The first thing that I would like to try is to have the panic handler be a function exported with weak linkage, which means that the C code could choose to override the panic function by declaring a function with the same symbol name. Another potential thing would be to do #1439 and #2189 and then have I think that addresses static linking. To address DLLs, I'll have to experiment a little bit. I'm not sure how external function references work in DLLs when they are provided by the loading module. |
Panic test runner starts itself as child process with the test function index representing a test block. The custom panic handler compares a global previously written string. If the panic message is expected, the success count is increased and the next test block is run. On failure the parent process prints collected results, before it terminates. This means, that only one @Panic is possible within a test block and that no follow-up code after the @Panic in the test block can be run. Depends on ziglang#14152. Closes ziglang#1356.
I know most people are not inclined to panic recovery, but is there consensus on this?
|
Panics are used in Kernels in non-recoverable situations, see https://en.wikipedia.org/wiki/Kernel_panic. So the only safe thing is to crash or go into "a known safe mode". Now what the "safe mode" or state would be for Zig and how to cleanly free all resources (in all cases/in supported cases) without making the language = compiler unnecessary complex is the question, you could think about.
What options do we have? We can either make 1. panic return or 2. panic longjump to a predefined "safe source location" and "do some magic behind the programmers back". Option 1: Option 2: 2.2 If we would want to ignore memory leaks tested in follow-up locations, but continue execution after panic check we can now either:
This would eliminate the very definition of reaching panics in the first place, which is to specify the detection of unrecoverable state. panics dont provide a way to specify where the safe state has been, as this could also be coded/used by the programmer in the error system. |
Option 2.2.2 would mean having potentially unbounded process spawns and memory consumption at the same time, because the user might mess up the test code. (very bad) EDIT: Numbering messed up, but the gist should be understandable. If not, let me know. |
I'm sorry, but I'm not familiar with internals of zig itself. I'm asking specifically because panic is not always acceptable behavior and for Zig to be usable generally it must provide means to recover fully from panic or have ability to eliminate abort side effect (even if you provide hook, you cannot resume program normally unless you have OS support) First of all, Zig has issue lacking destructors which prevents ability to automatically clean resources so automatic recovery would impose requirement on destructors as otherwise you have a lot of nasty behavior due to essential destructors not being present (Seems like to be tackled by #782 with some hacky way and unclear how it would behave during stack unwinding as it is abnormal control flow) This mean that effectively, simplest option - panic recovery - is undesirable, unless done in conjunction with
I should elaborate that my question was in context of code outside of your control which can be:
Manual control over panic is only possible within own code. If Zig is not going to provide proper tools to restore program state after panic, then it would be good to at least have ability to mis-compile if panic is present in code P.S. of course there is hard way to not use standard library or third party code in general 😄 |
Andrew said, that he had some ideas (for pointer to stack memory, not sure if thats https://gist.github.com/PeyTy/7983dd85026cc061980a6a966bc1afc2), but the overall issue with #782 is that it is unfeasible to compute all possible paths to ensure proper cleanup happens (and its a huge compilation time cost) and then you are stuck with either affine memory ownership semantics (Rust) or linear ones + all the edge cases not handled by such system. Rust for example can introduce memory leaks.
That does not fix the core of the problem: Code, which panics may not have the cleanup handled in any way, because the user could assume the process crashes and the Kernel does the cleanup.
|
As users it should be our responsibility to ensure that destructor code doesn't panic which would screw up stack unwinding even further.
Do you mean that user didn't set up
Not necessary to be stack analysis, but even simplest way to disallow linking panic handler would be nice |
I was hoping that the below might do the trick, but it seems that panic is always referenced. Presumably because it's referenced directly from the compiler rather than by normal mechanisms? const std = @import("std");
pub fn panic(msg: []const u8, error_return_trace: ?*std.builtin.StackTrace) noreturn {
_ = msg;
_ = error_return_trace;
@compileError("panic");
}
pub fn main() void {} |
Yes, it should not work, compiler should still be able to compile code even if it is never used as it needs to verify validity of your code The way I did it in Rust is by introducing destructor function that links invalid item and it would be eliminated during optimization if panic never happened. |
Huh, yeah, that works (EDIT only in release-fast - it still fails to compile in release-safe): const std = @import("std");
extern fn extern_panic() void;
pub fn panic(msg: []const u8, error_return_trace: ?*std.builtin.StackTrace) noreturn {
_ = msg;
_ = error_return_trace;
extern_panic();
std.os.exit(0);
}
pub fn main() void {
//@panic("oh no");
}
|
Panic test runner starts itself as child process with the test function index representing a test block. The custom panic handler compares a global previously written string. If the panic message is expected, the success count is increased and the next test block is run. On failure the parent process prints collected results, before it terminates. This means, that only one @Panic is possible within a test block and that no follow-up code after the @Panic in the test block can be run. Depends on ziglang#14152. Closes ziglang#1356.
This is a small change to help when reading failure logs which makes the "exited with code 1" and similar message include the test name. Further enhancements could do the following: * even if one unit test crashes the process, the parent process continues running the other unit tests * ability to test for expected panics (#1356) * timeouts on individual tests
Panic test runner starts itself as child process with the test function index representing a test block. The custom panic handler compares a global previously written string. If the panic message is expected, the success count is increased and the next test block is run. On failure the parent process prints collected results, before it terminates. This means, that only one @Panic is possible within a test block and that no follow-up code after the @Panic in the test block can be run. Depends on ziglang#14152. Closes ziglang#1356.
The user can use std.testing.spawnExpectPanic() in a test to spawn a child process, which must panic or the test fails. Internally, - 1. is_panic_parentproc is set from the cli arguments for simple reproduction of both test spawn and panic behavior, - 2. panic_msg is set as threadlocal, if comptime-detectable capabilities exist, to enable multithreaded processing and user-customized messages, - 3. error.SpawnZigTest is returned to the test_runner.zig - 4. the test_runner spawns a child_process on correct usage - 5. the child_process expected to panic executes only one test block This means, that only one @Panic is possible within a test block and that no follow-up code after the @Panic in the test block can be run. This commit does not add the panic tests to the server yet, since there are open design questions how many processes should be spawned at the same time and how to manage time quotas to prevent unnecessary slowdowns. Work on ziglang#1356.
The user can use std.testing.spawnExpectPanic() in a test to spawn a child process, which must panic or the test fails. Internally, - 1. is_panic_parentproc is set from the cli arguments for simple reproduction of both test spawn and panic behavior, - 2. panic_msg is set as threadlocal, if comptime-detectable capabilities exist, to enable multithreaded processing and user-customized messages, - 3. error.SpawnZigTest is returned to the test_runner.zig - 4. the test_runner spawns a child_process on correct usage - 5. the child_process expected to panic executes only one test block This means, that only one @Panic is possible within a test block and that no follow-up code after the @Panic in the test block can be run. This commit does not add the panic test capability to the server yet, since there are open design questions how many processes should be spawned at the same time and how to manage time quotas to prevent unnecessary slowdowns. Supersedes ziglang#14351. Work on ziglang#1356.
The user can use std.testing.spawnExpectPanic() in a test to spawn a child process, which must panic or the test fails. Internally, - 1. is_panic_parentproc is set from the cli arguments for simple reproduction of both test spawn and panic behavior, - 2. panic_msg is set as threadlocal, if comptime-detectable capabilities exist, to enable multithreaded processing and user-customized messages, - 3. error.SpawnZigTest is returned to the test_runner.zig - 4. the test_runner spawns a child_process on correct usage - 5. the child_process expected to panic executes only one test block This means, that only one @Panic is possible within a test block and that no follow-up code after the @Panic in the test block can be run. This commit does not add the panic test capability to the server yet, since there are open design questions how many processes should be spawned at the same time and how to manage time quotas to prevent unnecessary slowdowns. Supersedes ziglang#14351. Work on ziglang#1356.
The user can use std.testing.spawnExpectPanic() in a test to spawn a child process, which must panic or the test fails. Internally, - 1. is_panic_parentproc is set from the cli arguments for simple reproduction of both test spawn and panic behavior, - 2. panic_msg is set as threadlocal, if comptime-detectable capabilities exist, to enable multithreaded processing and user-customized messages, - 3. error.SpawnZigTest is returned to the test_runner.zig - 4. the test_runner spawns a child_process on correct usage - 5. the child_process expected to panic executes only one test block This means, that only one @Panic is possible within a test block and that no follow-up code after the @Panic in the test block can be run. This commit does not add the panic test capability to the server yet, since there are open design questions how many processes should be spawned at the same time and how to manage time quotas to prevent unnecessary slowdowns. Supersedes ziglang#14351. Work on ziglang#1356.
The user can use std.testing.spawnExpectPanic() in a test to spawn a child process, which must panic or the test fails. Internally, - 1. is_panic_parentproc is set from the cli arguments for simple reproduction of both test spawn and panic behavior, - 2. panic_msg is set as threadlocal, if comptime-detectable capabilities exist, to enable multithreaded processing and user-customized messages, - 3. error.SpawnZigTest is returned to the test_runner.zig - 4. the test_runner spawns a child_process on correct usage - 5. the child_process expected to panic executes only one test block This means, that only one @Panic is possible within a test block and that no follow-up code after the @Panic in the test block can be run. This commit does not add the panic test capability to the server yet, since there are open design questions how many processes should be spawned at the same time and how to manage time quotas to prevent unnecessary slowdowns. Supersedes ziglang#14351. Work on ziglang#1356.
The user can use std.testing.spawnExpectPanic() in a test to spawn a child process, which must panic or the test fails. Internally, - 1. is_panic_parentproc is set from the cli arguments for simple reproduction of both test spawn and panic behavior, - 2. panic_msg is set as threadlocal, if comptime-detectable capabilities exist, to enable multithreaded processing and user-customized messages, - 3. error.SpawnZigTest is returned to the test_runner.zig - 4. the test_runner spawns a child_process on correct usage - 5. the child_process expected to panic executes only one test block This means, that only one @Panic is possible within a test block and that no follow-up code after the @Panic in the test block can be run. This commit does not add the panic test capability to the server yet, since there are open design questions how many processes should be spawned at the same time and how to manage time quotas to prevent unnecessary slowdowns. Supersedes ziglang#14351. Work on ziglang#1356.
The user can use std.testing.spawnExpectPanic() in a test to spawn a child process, which must panic or the test fails. Internally, - 1. is_panic_parentproc is set from the cli arguments for simple reproduction of both test spawn and panic behavior, - 2. panic_msg is set as threadlocal, if comptime-detectable capabilities exist, to enable multithreaded processing and user-customized messages, - 3. error.SpawnZigTest is returned to the test_runner.zig - 4. the test_runner spawns a child_process on correct usage - 5. the child_process expected to panic executes only one test block This means, that only one @Panic is possible within a test block and that no follow-up code after the @Panic in the test block can be run. This commit does not add the panic test capability to the server yet, since there are open design questions how many processes should be spawned at the same time and how to manage time quotas to prevent unnecessary slowdowns. Supersedes ziglang#14351. Work on ziglang#1356.
The user can use std.testing.spawnExpectPanic() in a test to spawn a child process, which must panic or the test fails. Internally, - 1. is_panic_parentproc is set from the cli arguments for simple reproduction of both test spawn and panic behavior, - 2. panic_msg is set as threadlocal, if comptime-detectable capabilities exist, to enable multithreaded processing and user-customized messages, - 3. error.SpawnZigTest is returned to the test_runner.zig - 4. the test_runner spawns a child_process on correct usage - 5. the child_process expected to panic executes only one test block This means, that only one @Panic is possible within a test block and that no follow-up code after the @Panic in the test block can be run. This commit does not add the panic test capability to the server yet, since there are open design questions how many processes should be spawned at the same time and how to manage time quotas to prevent unnecessary slowdowns. Supersedes ziglang#14351. Work on ziglang#1356.
The user can use std.testing.spawnExpectPanic() in a test to spawn a child process, which must panic or the test fails. Internally, - 1. is_panic_parentproc is set from the cli arguments for simple reproduction of both test spawn and panic behavior, - 2. panic_msg is set as threadlocal, if comptime-detectable capabilities exist, to enable multithreaded processing and user-customized messages, - 3. error.SpawnZigTest is returned to the test_runner.zig - 4. the test_runner spawns a child_process on correct usage - 5. the child_process expected to panic executes only one test block This means, that only one @Panic is possible within a test block and that no follow-up code after the @Panic in the test block can be run. This commit does not add the panic test capability to the server yet, since there are open design questions how many processes should be spawned at the same time and how to manage time quotas to prevent unnecessary slowdowns. Supersedes ziglang#14351. Work on ziglang#1356.
When I add safety checks to code (e.g. a panic that fires if the API user violates their end of the API contract), I want to be able to test those safety checks (e.g. by intentionally violating contract in the test, and checking that we get the expected panic). However, there doesn't seem to be any way to catch panics in Zig, even with a custom panic handler (which has to return
noreturn
, and thus cannot swallow the panic, or otherwise not halt/hang the program, as far as I can tell?), so this kind of testing doesn't seem to be possible. For comparison, Rust tests support this via the#[should_panic(expected = "substr")]
annotation.Some possible proposals:
Add a general mechanism for catching panics. Go has this in the form of
recover
, and Rust hascatch_unwind
. Zig could have a builtin to do a similar job. This builtin would be usable anywhere, but explicitly anti-recommended for ordinary error handling.Provide a
test
-block-specific way to expect panics. Zig could have a builtin (e.g.@expectPanic(comptime expected_message_substr: []const u8)
that would apply to the scope of thetest
block, and would cause the test to succeed if it panics with a matching message, or fail if it panics with the wrong message or doesn't panic at all.Don't add this feature at all, and live with the fact that safety checks have to be tested at a higher level (e.g. by running each expect-panic-test as a separate subprocess and expecting that subprocess to crash, e.g. how
{#code_begin|test_err|expected message#}
inlangref.html.in
appears to work). Perhaps some mechanism could be added tostd.build
to make this a bit easier.My personal preference would be for (2), since that makes the tests very easy to write. If we want (1) as a more general mechanism, then perhaps we could still have (2), or something like it (possibly in userspace), as a convenience. If we don't want to support catching panics, then (3) could work, but I worry that that would make writing safety tests painful enough that library authors would be discouraged from writing them.
The text was updated successfully, but these errors were encountered: