godot icon indicating copy to clipboard operation
godot copied to clipboard

Show a GUI alert when crashing on desktop platforms

Open Calinou opened this issue 3 years ago • 18 comments

This makes it possible to know that Godot has crashed without looking at the console (which may not be visible to the user).

  • Change the window title while dumping the backtrace, as this can take a while to finish.
  • If file logging is enabled, print path to log file to standard error and the GUI alert.
  • Clicking OK will open the log file in the default program associated with .log files. Notepad has native associations for .log on Windows, so this will work on Windows too (once this PR is ported to Windows).

  • This closes https://github.com/godotengine/godot-proposals/issues/7917.

Preview

Linux

https://user-images.githubusercontent.com/180032/173110079-8756207b-cb3d-4b31-af61-6ccf0d690129.mp4

Windows

image

macOS

Screenshot_20240206_195358 Screenshot_20240206_195417

TODO

  • [ ] Consider adding a headless handler for OS::alert(), so we don't have to hack around it here. Make this handler print the string with some special formatting around it, but without the blocking behavior (as this behavior is undesired in headless mode).
  • [x] Port changes to Windows. Everything seems to work there 🙂
  • [x] Port changes to macOS. This is now functional thanks to @bruvzg, but https://github.com/godotengine/godot/pull/87842 needs to be merged if we want it to work with OS.crash() (the method used for testing this crash handler).
  • [ ] Redo for 3.x once everything is OK.

Production edit: closes godotengine/godot-roadmap#5

Calinou avatar Jun 10 '22 16:06 Calinou

I think that there should be an option to disable this(of course if it will be enabled by default). Not always is possible to use in CI headless build, so using normal build will just freeze entire CI job after crash

qarmin avatar Jun 11 '22 19:06 qarmin

Will this apply to Android editor port?

Zireael07 avatar May 21 '23 08:05 Zireael07

Will this apply to Android editor port?

No, as Android/iOS/HTML5 currently lack a crash handler.

Calinou avatar May 21 '23 18:05 Calinou

Port changes to macOS and Windows' crash handlers.

Probably won't be straightforward to do, new windows won't show (or at least won't be responsive) since OS event loop is not running while in the crash handler.

bruvzg avatar May 22 '23 06:05 bruvzg

Probably won't be straightforward to do, new windows won't show (or at least won't be responsive) since OS event loop is not running while in the crash handler.

By the way, it looks like the crash backtrace for the running project appears in the editor Output panel on macOS: https://github.com/godotengine/godot/issues/79194#issuecomment-1817485980

I've never seen this occur on Windows and Linux though. This could be a decent workaround for projects run from the editor if we can't make the GUI alert dialog show up on Windows and macOS.

Another alternative is to open the log file directly using OS::shell_open() on crash after the backtrace is done writing to that file.

Calinou avatar Nov 18 '23 12:11 Calinou

Hey, is this currently implemented into Godot or still being worked on? I would find this very useful.

TheFoxKnocks avatar Jan 13 '24 04:01 TheFoxKnocks

This PR hasn't been merged so it isn't implemented in Godot yet.

Zireael07 avatar Jan 13 '24 11:01 Zireael07

To avoid issues with non-responsive or missing GUI window, I would probably do something like this instead:

  • Add a new --crash-handler crashed_process_pid log_path command line argument that will display a window (a proper UI, not OS::alert) with the info.
  • Spawn a new process as soon as crash handler is triggered (since trace generation can take a few seconds, and spawned precess can display a responsive generating backtrace message while waiting for the original process to exit).

bruvzg avatar Feb 02 '24 07:02 bruvzg

To avoid issues with non-responsive or missing GUI window, I would probably do something like this instead:

I agree it's the way to go, but it's a lot of effort to do in a polished manner. I wouldn't be able to finish for 4.3… so maybe I should just make it open a log file on macOS and be done with it. Windows/Linux have functional GUI crash handlers already, though I don't see a way to do it for Android either (not with the suggested approach either).

Crashing without any visible GUI feedback is pretty bad, so we should do something about it as soon as possible even if the implementation isn't ideal.

Calinou avatar Feb 06 '24 16:02 Calinou

As a workaround it's possible to display dialog via Apple script, shell_open also won't work outside normal app and should be replaced with script call as well.

 		fprintf(stderr, "\nFind the log file for this session at:\n%s\n\n", log_path.utf8().get_data());
 
 		if (DisplayServer::get_singleton()->get_name() != "headless") {
-			OS::get_singleton()->alert(vformat("%s: Program crashed with signal %d.\n\nFind the log file for this session at:\n%s\n\nClicking OK will open this log file. Please include the this log file's contents in bug reports.", __FUNCTION__, sig, log_path));
-			OS::get_singleton()->shell_open(log_path.utf8().get_data());
+			List<String> args;
+			args.push_back("-e");
+			args.push_back(vformat("display alert \"Crash\" message \"%s: Program crashed with signal %d.\\n\\nFind the log file for this session at:\\n%s\\n\\nClicking OK will open this log file. Please include the this log file's contents in bug reports.\"", __FUNCTION__, sig, log_path.c_escape()));
+			OS::get_singleton()->execute("osascript", args);
+			args.clear();
+			args.push_back(log_path);
+			OS::get_singleton()->execute("open", args);
 		}
 	} else if (DisplayServer::get_singleton()->get_name() != "headless") {
-		OS::get_singleton()->alert(vformat("%s: Program crashed with signal %d.", __FUNCTION__, sig));
+		List<String> args;
+		args.push_back("-e");
+		args.push_back(vformat("display alert \"Crash\" message \"%s: Program crashed with signal %d.\"", __FUNCTION__, sig));
+		OS::get_singleton()->execute("osascript", args);
 	}
 
 	// Abort to pass the error to the OS

bruvzg avatar Feb 06 '24 17:02 bruvzg

~~It seems I accidentally force-pushed changes that removed Windows support last week, so I'll need to restore them before this PR can be merged.~~ Edit: Fixed.

As a workaround it's possible to display dialog via Apple script, shell_open also won't work outside normal app and should be replaced with script call as well.

It works great, thanks :slightly_smiling_face: I've added you as a co-author.

Calinou avatar Feb 06 '24 19:02 Calinou

Crashing without any visible GUI feedback is pretty bad, so we should do something about it as soon as possible even if the implementation isn't ideal.

Just wanted to say I completely agree. Coming from GameMaker, the lack of a crash report popup was a surprise and has made tracking down bugs people find significantly more challenging, whereas GameMaker would point to the script where it crashed, and even if it's not always 100% accurate, it's still good guidance for tracking down the problem.

TheFoxKnocks avatar Feb 06 '24 19:02 TheFoxKnocks

whereas GameMaker would point to the script where it crashed, and even if it's not always 100% accurate, it's still good guidance for tracking down the problem.

Note that like the existing command line crash handler, this GUI crash handler will only point you to the cause of the error if you're using a build with debug symbols. Official binaries don't include debugging symbols for binary size reasons (and they currently can't be downloaded separately).

Also, even with debug symbols, it will only point you to the C++ source of the error, not your script line. Pointing to the script line would require something like https://github.com/godotengine/godot-proposals/issues/963.

Calinou avatar Feb 06 '24 19:02 Calinou

Wowzie, this stuff is great! No idea how I missed it, sorry.

Anyways, I wonder, what happens if any of the intermediate steps causes an exception on its own? Would it just stop there, recall the handler and cause an infinite loop, or something else?

deralmas avatar Feb 06 '24 23:02 deralmas

Anyways, I wonder, what happens if any of the intermediate steps causes an exception on its own? Would it just stop there, recall the handler and cause an infinite loop, or something else?

I've never seen the crash handler call itself, so I think it'll just fall back to OS behavior and exit in this case.

Calinou avatar Feb 07 '24 00:02 Calinou

Anyways, I wonder, what happens if any of the intermediate steps causes an exception on its own? Would it just stop there, recall the handler and cause an infinite loop, or something else?

The first thing our crash handler do, is restoring default signal handlers. So if it crashes, it would behave as if there is no custom crash handler.

bruvzg avatar Feb 07 '24 05:02 bruvzg

Also, clicking the X instead of the 'Ok' should not open the log file, users should not have to open it.

Unfortunately, we have no control over this because alert() returns void. It doesn't return whether the OK button was pressed or whether it was closed through other means.

Making alert() return another type would break compatibility, so a new method would need to be added.

Calinou avatar Mar 19 '24 22:03 Calinou

Rebased and tested again on Linux, it works as expected.

Calinou avatar Oct 04 '24 18:10 Calinou