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

Print exception stack trace in CsPackageManager.TryRun #229

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

Honkbinger
Copy link

This is not very helpful
Screenshot_4
I suggest replacing it with at least this
Screenshot_3

Notes

You can just print e, but it's a bit ugly:
Screenshot_5
Idk how to capture line number
Also, in theory there's a simpler method:

StackTrace st = new StackTrace(e);
st = new StackTrace(st.GetFrames().SkipLast(1));

But it's not available in .NET 6 :BaroDev:

@Honkbinger
Copy link
Author

Honkbinger commented Sep 28, 2024

At first i thought inner exception could never happen and is useless but now i found that exceptions in static constructors could trigger it
I added some prints but i didn't test them yet
I obviously tested it by just replacing TryRun with harmony, but i can't do that for inner exception because it happens before my testing mod initialize :BaroDev:
So i guess i need to compile luatrauma to test it, and it'll probably take me a few hours to figure out how to compile it in VSCode
:BaroDev:

EDIT: seems working
Screenshot_3

@MapleWheels
Copy link
Collaborator

Hi Honk,

This is a good PR. I won't be able to merge it as we're doing a big code refactor. However, I will be manually implementing this into the Runner. Thanks for the contribution.

I'm going to leave the PR open for now and will close it once implemented.

Finally was able to compile luatrauma
And stack trace seems to be 1 frame longer than in harmony patch
also you need System.Diagnostics (kek)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants