godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.16k stars 97 forks source link

Add global crash handler/logger #1896

Open lxknvlk opened 3 years ago

lxknvlk commented 3 years ago

Describe the project you are working on

A game in godot

Describe the problem or limitation you are having in your project

  1. Users having crashes that i can not locate
  2. Exported game crashes, runs ok from editor

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Such implementation would allow to add crash logging so we can debug the crashes on exported versions. Also possibility to add crash reporter like firebase crashlytics.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Not sure how it can work, but such a feature is mandatory for any mature game engine. Probably some global try-catch?

If this enhancement will not be used often, can it be worked around with a few lines of script?

No possible workarounds. One workaround that comes in mind is to use Unity engine.

Is there a reason why this should be core and not an add-on in the asset library?

Yes! This is a must have feature for all game engines! (All other cool guys do it!)

Calinou commented 3 years ago

Such implementation would allow to add crash logging so we can debug the crashes on exported versions.

Log files are already written by default on desktop platforms, but this is disabled on mobile platforms. Also, log files are only written if the project exits normally (without crashing), since the log file isn't constantly flushed to avoid performance issues.

Edit: It's now possible to enable stdout flushing on every line in release builds, but this has to be enabled manually in the project settings.

See https://github.com/godotengine/godot/pull/22778 which implements a crash reporting system, but it's not a customizable handler.

Probably some global try-catch?

C# has exceptions, but GDScript doesn't :slightly_smiling_face:

I'm not sure exactly how we could register a GDScript method to be called when crashing. I agree this would be a good solution as it's the most extensible one (compared to supporting a single crash reporting service).

Yes! This is a must have feature for all game engines! (All other cool guys do it!)

Just saying, but this is not a sufficient reason in itself to add a feature to the core engine. When opening a proposal, there needs to be more to it than "other engines do it".

lxknvlk commented 3 years ago

Any progress on this most important feature?

Calinou commented 3 years ago

Any progress on this most important feature?

As far as I know, nobody has started working on this feature yet. It's not exactly easy to implement given how Godot works.

hhyyrylainen commented 3 years ago

Would something like Google Breakpad be something that could be considered to be integrated into Godot (just the crash dump generation part)? I ported a game over to Godot and we used to use a custom engine that I had integrated Breakpad into. It allowed us to (with an additional launcher component for uploading the dumps) receive native stack traces for where users experienced crashes, and that allowed us to narrow down and fix many issues we weren't able to reproduce (only getting the occasional crash reports). With Godot now, for most crash reports all I can say is that "if you can get the crash to happen consistently we can do something about it", which doesn't feel too good to not be able to help users who experience crashes. For many users they don't even get any other info than "process exited with code 1" and that gives us literally 0 info on what might have gone wrong.

I'm asking here as with a quick search around this seems the most up to date / relevant discussion related to what I'd want to have in Godot.

Calinou commented 3 years ago

Would something like Google Breakpad be something that could be considered to be integrated into Godot (just the crash dump generation part)?

As per https://github.com/godotengine/godot/pull/22778, there are no plans to add a crash reporting system in core Godot. Unfortunately, given a crash handler needs to hook to OS signals, it can't be done with GDNative (and most likely not with GDExtension as well).

Godot already has a custom crash handler system (platform/{windows,osx,linuxbsd}/crash_handler_*), but this crash log isn't written to a file yet. Doing so should be fairly easy for anyone interested (writing logs/crash_godot_{date}_{time}.log in user://). However, for crash backtraces to be useful, your builds need to include debugging symbols. Official builds don't include debugging symbols to keep file sizes low.

hhyyrylainen commented 3 years ago

Do non-debugging symbol containing builds also output some kind of crash handler info? Because for me basically all crash reports from users don't have any kind of crash trace. As we provide a launcher that most users use that shows the STDOUT (and probably STDERR, I won't say I'm 100% certain on that) of the game process, it seems to be completely missing. The game just ends with an entirely looking normal log message. I think I have something like 20-30 crash reports on hand like that, where I just have to say that I can't help.

A huge reason to use breakpad is that you can keep builds small, but still have line numbers back in stracktraces when you process them. So Godot official builds could be built with debug symbols, extracted to the breakpad symbol files, stripped, and then distributed. The breakpad symbol files could then be distributed separately to developers wanting to decode crash dumps. Or at least make it easy to make custom builds that leave you with the breakpad symbol files and a stripped Godot executable.

Reading the conclusion of that PR it seems here wouldn't be a fundamental problem with that kind of build system change at least being merged to the main repo (small end user downloads, and useful stack traces from any native crash for developers).

Edit: in case I wasn't clear, I don't actually really need Godot to handle uploading the crashes or anything like that. Just creating the minidump to a known folder would be enough for me to build out the workflow users would use to then consent to uploading the crash dump to our custom site.

Calinou commented 3 years ago

Do non-debugging symbol containing builds also output some kind of crash handler info?

They do, but the crash stacktraces will be meaningless.

Because for me basically all crash reports from users don't have any kind of crash trace. As we provide a launcher that most users use that shows the STDOUT (and probably STDERR, I won't say I'm 100% certain on that) of the game process, it seems to be completely missing. The game just ends with an entirely looking normal log message. I think I have something like 20-30 crash reports on hand like that, where I just have to say that I can't help.

The crash handler currently doesn't write to log files if file logging is enabled. Instead, it will print to the console if you started the project from a terminal (or the command prompt on Windows, for projects exported in debug mode). That's what I was referring to above.

So Godot official builds could be built with debug symbols, extracted to the breakpad symbol files, stripped, and then distributed.

Official Godot builds are compiled with GCC/MinGW on Windows, and link-time optimization is used on all platforms. How friendly is Breakpad with MinGW and LTO? I remember LTO not working well with debugging symbols at the same time, but that may have changed since the last time I checked (2017). This is another reason we've refrained from distributing official builds with debugging symbols; we'd like performance to be identical to release builds if possible. Separate slow debug builds are an option, but they aren't viable for most games unless you distribute them using a separate Steam channel or something along those lines.

There's a separate_debug_symbols=yes SCons option you can use to have separate debug symbols in .debug files when compiling Godot with GCC or Clang on Windows, macOS and Linux. (MSVC already uses separate .pdb files if you pass debug_symbols=yes for target=release builds.)

hhyyrylainen commented 3 years ago

for projects exported in debug mode

Could that be it? Is it then the case that as we always export in non-debug mode, our end users who experience crashes currently not seeing any tracebacks, is the expected behaviour? That's not very optimal experience for a game. I can understand development tools having pretty convoluted bug reporting ways, but for end users it needs to be as simple as running the normal build and if it crashes then looking in the logs to provide that to the game developers.

Official Godot builds are compiled with GCC/MinGW on Windows, and link-time optimization is used on all platforms. How friendly is Breakpad with MinGW and LTO? I remember LTO not working well with debugging symbols at the same time, but that may have changed since the last time I checked (2017).

My understanding is that LTO obviously loses some information, like how compiling in release mode might inline functions. But I think that if you give the LTO compiled version with debug symbols to breakpad it would be able to extract valid line number mappings, though again LTO can have inlined stuff and things like that. But I have to admit that when I last used breakpad I didn't use LTO, but I did use full release mode to compile the game, and many of the crash callstacks we got from users did lack many details, but they still gave a lot of clues as to what might be wrong.

Regarding this I found a post on stackoverflow: https://stackoverflow.com/a/54508901/4371508 from 2019 saying that GCC 8 now supports LTO and debug symbols compiling flags at the same time.

There's a separate_debug_symbols=yes SCons option you can use to have separate debug symbols in .debug files when compiling Godot with GCC or Clang on Windows, macOS and Linux. (MSVC already uses separate .pdb files if you pass debug_symbols=yes for target=release builds.)

What are these used for? pdb files are fine for breakpad on Windows but I think unstripped (meaning with debug symbols) executable and .so files are what you feed to it on Linux to create the breakpad symbol files.

Calinou commented 3 years ago

our end users who experience crashes currently not seeing any tracebacks, is the expected behaviour?

Indeed. You could compile your own release export templates with the windows_subsystem=console SCons option to make the command prompt appear even for a project exported in release mode. However, this doesn't lead to great UX since the command prompt can be closed accidentally by the user, which will close the project without notice. Even worse, selecting text within the command prompt will cause the project to freeze until the user presses Enter or Escape in the command prompt due to Windows console selection behavior.

Some game engines went as far as implementing their own console subsystem on top of the Windows GUI subsystem just because of this. If you've ever seen a crash dialog in an id Tech-based game, you know what I'm talking about :slightly_smiling_face:

What are these used for? pdb files are fine for breakpad on Windows but I think unstripped (meaning with debug symbols) executable and .so files are what you feed to it on Linux to create the breakpad symbol files.

.debug files are just a way to store debug symbols separately, like you'd do on Windows with .pdb files. It's pretty much standard on UNIX systems by now, although its use is not very common. When you use separate debug symbols, the main binary no longer contains debug symbols, which allows you to move them away and move them back easily (unlike strip which is a one-way operation).

hhyyrylainen commented 3 years ago

.debug files are just a way to store debug symbols separately, like you'd do on Windows with .pdb files. It's pretty much standard on UNIX systems by now, although its use is not very common. When you use separate debug symbols, the main binary no longer contains debug symbols, which allows you to move them away and move them back easily (unlike strip which is a one-way operation).

That does sound pretty handy. I guess we don't know before we try if the breakpad symbol extraction tool works with those or not. As I said I've only used it on full executables and .so files with embedded debug symbols.

bitbrain commented 3 years ago

Any reason why https://github.com/godotengine/godot/pull/22778 did not make it in? It looks exactly like something that would solve this.

Calinou commented 3 years ago

Any reason why godotengine/godot#22778 did not make it in? It looks exactly like something that would solve this.

While it's a good start, there are a few issues I can see with that implementation:

I'd prefer a solution that would allow developers to locally "reverse" crash backtraces to useful backtraces using debug symbols. It requires manual user reports, but it won't require any hosting. It's still an upgrade over what we have now :slightly_smiling_face:

On Android, there are already specific tools in place that can be used on the Google Play side. We could plug those into Godot as well, but it's quite a lot of work to implement.

hhyyrylainen commented 2 years ago

I'm going to now start really looking into this. I think initially I'll just try to get the crash dump generator as an extra module added to the Godot build and make sure it works (and probably I'll set it to be disabled by default). I think due to my unfamiliarity with the Godot build process (I did get the official docker based build working), I'll be content with having external scripts that prepare the Godot engine symbol files (building and running the symbol dumper requires Google depot tools, for the crash dump library I think I'll manually need to write the minimum required scons files to make it work).

It seems that the build.sh script does not generate debug symbols by default at all. When setting the extra file debug info generation I got the extra file, but breakpad symbol dumper doesn't understand it, just fails with a general can't dump symbols error. I found editing build.sh to compile with debug_symbols=yes separate_debug_symbols=no use_lto=yes use_static_cpp=yes seems to work perfectly to create an executable with debub symbols I can dump and then strip it. Funnily enough that ends up with a smaller executable than the default build (which seems to have some public symbols left in it):

normal:
57M godot.x11.opt.64.mono
60M godot.x11.opt.debug.64.mono
with debug symbols:
489M    godot.x11.opt.64.mono
531M    godot.x11.opt.debug.64.mono
debug symbol builds after strip:
49M godot.x11.opt.64.mono
51M godot.x11.opt.debug.64.mono

Exported game still seems to work perfectly fine with the stripped executable, so I assume the public symbols weren't important.

hhyyrylainen commented 2 years ago

I've opened a PR for what I have so far: https://github.com/godotengine/godot/pull/56014 And besides making Breakpad compile with SCons and finding a clean way to hook into the existing crash reporter code in Godot, it doesn't seem too hard. Of course that's just the crash dumping part. The symbol file generation, upload, and stackwalking require (external) scripts on top of that.

marko995 commented 1 year ago

Hi guys, any progress on this?

Calinou commented 1 year ago

Hi guys, any progress on this?

There is https://github.com/godotengine/godot/pull/56014 if you need Breakpad integration, but I don't think anyone has started work on a "scriptable" crash handler you can override from GDScript.

hhyyrylainen commented 1 year ago

Yeah, my approach (https://github.com/godotengine/godot/pull/56014) just adds the crash dump creation capability to Godot. To implement a full crash reporting pipeline, there's a separate launcher program that detects the created crash dumps and facilitates uploading them to a server. I thought that I'd have a better chance to get just the core critical crash dump creation piece, which is absolutely required as the base for any kind of user crash reporting feature, merged into Godot much faster than try to integrate the full crash uploading feature as well somehow.

marko995 commented 1 year ago

@hhyyrylainen This won't work for mobile games :/ crashlytics is a must-have for every little bit of serious game, and crash logger can be game changer for godot, I think.

hhyyrylainen commented 1 year ago

Sadly my focus is not making sure this (edit: I thought I was replying to a comment on my PR so I've worded this a bit awkwardly to fit this issue's context) works for mobile games as the game project I'm working on would be extremely difficult to port to mobile with the end result being that I'm not going to give it much thought at all for at least a decade. I do hope that the work here could be used as a base for also mobile crash reporting. But someone else will need to extend the functionality for that.

I saw the discussion about adding a crash reporting component to Godot (which in some form would be a crash report sender window that opens to give the player the option to send a report) and there didn't seem to be much consensus there so I've been hoping that this the bare minimum initial step would be easier to get merged. And then people could start discussing adding a tool inbuilt to Godot for sending those created crash dumps. But this is the bare minimum that needs to be integrated directly to the core engine to make external crash report sending tools possible to make at all.

marko995 commented 1 year ago

@hhyyrylainen that could work. Write crashes to a file, and send them to the server on the next startup. can you send a link?

hhyyrylainen commented 1 year ago

This is my pull request: https://github.com/godotengine/godot/pull/56014

Note that it hasn't been accepted, and I haven't fixed it for the latest Godot 4 code as I'm waiting to get the at least some kind of promise that if I update my code there'd be a chance to get it merged before I need to fix it again.

marko995 commented 1 year ago

another solution would be a global crash callback (like global try/catch), where you attach c++ code (or gdscipt) with functionality like: save to file or send to server immediately, close game etc.

How long will it take someone to do this?

marko995 commented 1 year ago

@hhyyrylainen if your PR does such a similar thing, can you write a few sentences of docs where can I start from your PR?

hhyyrylainen commented 1 year ago

According to Breakpad docs and other info I've read, it's a bad idea to try to do more than the bare minimum from the crash callback (breakpad provides this, I just use it to write the created crash dump to stdout). So trying to open a window or do a HTTP upload is not the most reliable option. The usual advice is to have a separate guard dog process that detects the crash or, like you said offer to upload the crash on the next start up.

My PR is pretty straightforward as long as you don't try to read any of the added breakpad files https://github.com/godotengine/godot/pull/56014/files just reading the first few files should show how its done pretty easily. I just hooked into where Godot initializes or skips its own callstack on crash printer (this is always disabled in exported games) to initialize breakpad (the breakpad initialize is basically just standard breakpad use). One complication is that I want to put the crashdumps in a Godot project specific folder so I need to update the dump setup when that info is available.

Anyway I guess basically if you want to do this yourself is you just need to put a call to initialize breakpad in the Godot main before the game data loading is. That way a bare minimum breakpad integration is achieved.

bitbrain commented 1 year ago

Would it be possible that instead of implementing a particular solution of crash handling into Godot, this proposal could provide a generic "crash handler API" that can be hooked into via GDExtension (or GDScript), so people can then provide various implementations of a crash handler (e.g. Breakpad).

By default, the Godot native crash handler could just output files to a crash report folder locally.

The reason why I am suggesting this is that such a generic contract would have a lesser likelihood of being rejected by the maintainers (in theory), so folks would be unblocked to provide their own GDExtension for it.

hhyyrylainen commented 1 year ago

Breakpad has to be attached as early in the process startup as possible. So for me it is a critical point that Breakpad is basically inserted into Godot's main function before basically any engine systems are initialized. This ensures that players whose computers Godot can't run on will still get crashes generated. Also Godot would have to reimplement the crash detection for each platform that Breakpad already contains.

By default, the Godot native crash handler could just output files to a crash report folder locally.

This is entirely what I implemented in that my PR I've already linked so many times.

The reason why I am suggesting this is that such a generic contract would have a lesser likelihood of being rejected by the maintainers (in theory), so folks would be unblocked to provide their own GDExtension for it.

Finally someone with the same opinion. This is exactly why I made just a barebones implementation. Still my PR has already been open for a year and a half and only at one point did I get someone to review it a bit.

marko995 commented 1 year ago

@hhyyrylainen I like the idea of using GDExtension to handle crashes. So can we go with this generic crash handler proposal? The problem with your PR is to too long, and has too many files :/ Is it possible to change your code where crash handler uses GDExtension instead of the exact implementation?

hhyyrylainen commented 1 year ago

Not unless someone wants to maintain a stripped down Breakpad version. I already only grabbed to my PR the files required for Breakpad crash generation. The PR is only like 8 files + the bare minimum library files needed from Breakpad to compile the crash dump generation parts for each platform. It doesn't include any of the tools for processing crash dumps or symbol files.

Ragueel commented 1 year ago

It is weird that godot team hasn’t implemented a solution for this yet. Usually post launch maintenance is very important for any game, especially in mobile market. Without tools like crashlytics or sentry I can’t imagine launching a successful project with godot.

Also I am not sure how it is supposed to work with GDScript if it has no concept of errors and you can’t throw it. There is push_error but it only outputs strings without passing some extra information. So new ways to log messages probably is needed.

Still hope that it will be added soon.

bitbrain commented 1 year ago

Also I am not sure how it is supposed to work with GDScript if it has no concept of errors and you can’t throw it.

By design, Godot does not let users "crash" the game in case an unhappy path is met (e.g. via Exceptions). The idea is that the game should just "keep running" and not suddenly stop or break. However, sometimes the underlying engine leads to a crash, so in that case Godot should still produce a "trace" of what happened. This is what this proposal is about, not trying to give developers ways to throw their own exceptions.

marko995 commented 1 year ago

Also I am not sure how it is supposed to work with GDScript if it has no concept of errors and you can’t throw it.

By design, Godot does not let users "crash" the game in case an unhappy path is met (e.g. via Exceptions). The idea is that the game should just "keep running" and not suddenly stop or break. However, sometimes the underlying engine leads to a crash, so in that case Godot should still produce a "trace" of what happened. This is what this proposal is about, not trying to give developers ways to throw their own exceptions.

I agree, but I would like to log somewhere that silent crashes.