Skip to content
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

Global panic hook + catch_unwind #120

Closed
wants to merge 9 commits into from
Closed

Global panic hook + catch_unwind #120

wants to merge 9 commits into from

Conversation

Mothblocks
Copy link
Member

Fixes #118.

Will log to rustg_panic.log, though I'm not sure where that's going to end up being.

@Mothblocks Mothblocks marked this pull request as draft October 24, 2022 21:06
@Mothblocks
Copy link
Member Author

Drafted so I can make output path configurable

@AffectedArc07
Copy link
Member

Dumb question, this is entirely "safe" and "rust native"?

IE - its not auxtools levels of process hooking or similar, its just normal stuff?

@Mothblocks
Copy link
Member Author

I forgot to finish this oops

Yes this is completely fine, ctor is a tiny hack but extremely battle tested and absolutely nowhere close to anything like auxtools

@Mothblocks
Copy link
Member Author

I'm not sure if this should be failing "silently" like it is now since that is going to cause problems, but that is how it is implemented right now 😓

Going to update this so that we can get a better grasp of errors on tg

@itsmeow
Copy link
Contributor

itsmeow commented Dec 27, 2023

@Mothblocks My suggestion is just make it up to the individual modules how to handle a panic unwind. My module currently returns the panic string directly, since it normally outputs JSON that is quite easy to test for.

Obviously for something like file_read it's a bit more difficult, but for now I think it's better than crashing the entire game.

@ZeWaka
Copy link
Contributor

ZeWaka commented Apr 21, 2024

Mostly implemented in #160, this is pretty abandoned these days.

@ZeWaka ZeWaka closed this Apr 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Panicing inside FFI is undefined behavior (and we have plenty of branches that panic)
5 participants