Crash reporter #87
Merged
Crash reporter #87
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The program's first argument is not guaranteed to be a path to the current executable. Also, at least on Linux, `std::env::current_exe()` is able to correctly find the new location of the executable if the user moves the executable before the panic handler runs.
Setting this to `forbid` seems to break clippy when using wasm-bindgen. I'm setting this to `warn` so we can see clippy lints for crates that depend on luminol-web in web builds.
…6f12f13605de26f214
Without this, the Git version will always have "-modified" at the end in web builds even if the code is unmodified, because the Trunk hooks modify .cargo/config.toml.
Luminol can now be run offline after loading it in a browser once.
This fixes a problem where sometimes, either cross-origin isolation would not be enabled at all or sw.js would have its own COEP (but not the COEP for other files) set to require-corp instead of credentialless. This would be fixed by refreshing the page a number of times. Now, COI should always be enabled and the COEP for sw.js should always be credentialless, without the user having to refresh the page: the page is automatically refreshed.
Haha, turns out we don't even need credentialless COEP to send no-cors requests! The require-corp COEP works just fine.
After further testing, it seems Firefox does require credentialless COEP for this type of no-cors request, so Chromium allowing it is probably a bug.
This fixes some edge cases I experienced when testing in Firefox where the user refreshing the page sometimes causes the page's COEP to revert to "require-corp" (the service worker is supposed to set it to "credentialless"). I hate web development so much.
This fixes a bug where updating one of the HTML or JavaScript files (which changes the output of the `luminol_core::version!` macro) doesn't cause every crate that uses the `luminol_core::version!` macro to be recompiled, resulting in parts of Luminol getting different Git revision strings from this macro.
5 tasks
somedevfox
requested changes
Jan 16, 2024
This prevents the Git revision from being miscalculated if some of the crates are at a different version than others.
|
Since I'd already changed how the Git version is calculated in web builds, I've also moved the Git version into the luminol crate (#89). This will prevent the version from being different in different crates if the crates are not all at the same revision. |
melody-rs
approved these changes
Jan 31, 2024
Since wgpu errors result in panics, this seems like a useful thing to have.
|
I can't merge this until @somedevfox approves |
melody-rs
approved these changes
Feb 1, 2024
somedevfox
approved these changes
Feb 2, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Connections
Description
Luminol now restarts after panicking on both native and web platforms. After restarting, it shows a window asking the user if they want to send the crash report to the developers. The crash report is a JSON object sent as a POST request; it contains the panic message as well as the Git revision, target triple and profile (debug/release).
It reports to http://localhost:3246 right now for easy testing. This should be changed in crates/ui/src/windows/reporter.rs to a real URL at some point. Don't worry about CORS - web builds can send crash reports even if the endpoint doesn't set CORS headers.
Restarting after panics is done on Unix-like systems by using one of the exec system calls to replace the current process without creating a new one. On Windows, it spawns a background process and then terminates the original one. For WebAssembly, it's done by reloading the WebAssembly modules. Reports are persisted across restarts in native builds by creating a temporary file and in web builds by saving the report as a JavaScript global variable.
I was originally going to try using
std::panic::catch_unwindto catch panics, but this doesn't work in WebAssembly due to the panic_abort panic handler and it causes segfaults and other signs of undefined behaviour in native builds somewhere inside of winit.There are also some related improvements to web builds:
Testing
Right now, a panic can be triggered by attempting to create or edit events in the map editor.
Testing for reception of reports on localhost can be done with a simple echo server like this one: https://github.com/cdfuller/echo-server. If you use this one in particular, make sure to run it with the
-vflag so that it prints the contents of requests. Whatever echo server is used, test that it is able to receive reports from both native and web builds of Luminol.It would also be a good idea to check that a real, internet-facing server is able to receive reports, especially that one that does not support CORS is able to receive reports from a web build of Luminol.
Checklist
cargo fmt.cargo clippy. If applicable, add:--target wasm32-unknown-unknowncargo build --releasetrunk build --release