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

Switch to mclo.gs crash report upload, replace GTNH-specific code with configuration, and fix stacktrace deobfuscator #11

Merged
merged 9 commits into from Jun 3, 2023

Conversation

wlhlm
Copy link
Member

@wlhlm wlhlm commented Jun 1, 2023

Welp, I just wanted to quickly switch the service to which crashes get uploaded to...😭

This includes multiple thematically different changes. I hope it's still reviewable in a single PR.

1. Update buildscript

BetterCrashes hasn't been changed in a while, so there was a bit of extra work to do outside of just updating build.gradle. Please note the changes to the other files in the "Update buildscript" commit.

2. Switch from paste.ubuntu.com to mclo.gs

BetterCrashes previously had the feature to upload crashlogs to paste.ubuntu.com. The problem with that service is that it requires a login in order to view the paste, which prompted me to seek an alternative service. My choice fell on mclo.gs which is used by PrismLauncher. It's a service made specifically for sharing MC crashlogs. It does some highlighting and concealing sensitive parts of the logs. API was very easy to work with.

Only drawback I found was that crashlogs are only stored for 90 days after the last view. Please let me know if you think that's a deal breaker.

Here is an example log on mclo.gs: https://mclo.gs/zd1mS7E

3. Replace GTNH-specific code with configuration

There previously was some GTNH-specific code that I didn't really see the merit of, so I replaced it with additional configuration parameters that make BetterCrashes more useable outside of GTNH.

There will be a followup PR that updates the mod config in the main modpack repo.

New config options:

  • crashLogPasteService: currently only a single paste service is supported, but it'll be very easy to support multiple if the need arises
  • issueTrackerURL: adds a button to the crash screen that takes the player to a bug tracker
  • stacktraceDeobfuscation: enables configuration of the integrated stacktrace deobfuscator
  • unsupportedMods: list of mods which the pack authors deem unsupported. This makes BetterCrashes prompt the player to make special mention of those mods in their bug report

4. Enable the stacktrace deobfuscator

BetterCrashes shipped with some functionality to deobfuscate stacktraces. Turns out that most of it was non-functional, so I fixed it. Due to the license of MCP, the mappings can't be redistributed with the mod itself and thus have to be downloaded at runtime. This updates the download location from the old defunct mcpbot location to Forge Maven and adds a hash check. The download will be triggered the first time the crash screen appears and the mapping will be saved in a file called bettercrashes-stackdeobfuscator-methods.csv in .minecraft.

wlhlm added 7 commits June 1, 2023 22:02
paste.ubuntu.com requires a login for viewing the paste and that is
annoying. mclo.gs is made for Minecraft logs. It does some special
highlighting and censors certain information that is not relevant.
Drawback is that logs are only valid for 90 days after last viewing.
@wlhlm wlhlm requested a review from miozune June 1, 2023 20:36
@wlhlm wlhlm requested a review from miozune June 2, 2023 15:52
@Dream-Master Dream-Master requested review from a team June 2, 2023 17:36
@wlhlm
Copy link
Member Author

wlhlm commented Jun 2, 2023

Thanks for the feedback! Should all have been addressed now.

@glowredman
Copy link
Member

One minor thing: You're applying the stable_12 mappings, which are not the ones used (by defaut) in the Forge MDK (MCP 9.05). These sadly were only ever distributed as part of the Forge MDK. Downloading the methods.csv file is easier however, as you can just use the URL: https://raw.githubusercontent.com/MinecraftForge/FML/1.7.10/conf/methods.csv.
Most mappings are the same but stable_12 has some new mappings, some mappings were changed and some even removed. In some cases this may lead to confusion when trying to patch the crash, unless one knows about this difference.
I'm indifferent how/if you change the code, I just wanted to point this out in case you weren't aware.

@wlhlm
Copy link
Member Author

wlhlm commented Jun 2, 2023

@glowredman Great feedback!

You're applying the stable_12 mappings

From my side, there was no reasoning for choosing those specific mappings other than that's what we use in our buildscripts. I will rely solely on your judgement--if you think a different version is more appropriate, then I'll change it.

Downloading the methods.csv file is easier however, as you can just use the URL: https://raw.githubusercontent.com/MinecraftForge/FML/1.7.10/conf/methods.csv.

Having that file available directly is a great suggestion, that'll allow me to throw out all the zip extraction code! In addition, it will save the Forge Maven some traffic. URL looks pretty stable to me as well. But as you mention, those are some older/different mappings.

I just wanted to point this out in case you weren't aware.

I was aware that the stable_12 mappings were newer, but I didn't know of the methods.csv in MinecraftForge GH. The thing is, if we as devs look at crashlogs from players and try to debug our mods, using the stable_12 mappings would make the most sense to me, as that is what our mods use through their buildscripts for deobfuscating MC.

Again, I'd like to rely on your judgement, so consider my points above and let me know what you think is the better approach.

@eigenraven
Copy link
Member

The buildscript can be a bit confusing wrt mappings, we use the forge embedded mappings for methods&classes, but stable_12 for parameter names.

@wlhlm
Copy link
Member Author

wlhlm commented Jun 2, 2023

Ahh OK. So then I don't see any downsides here (I can even simplify the code 😃) and will implement those changes.

This removes the necessity to extract a zip archive.
@wlhlm
Copy link
Member Author

wlhlm commented Jun 2, 2023

Updated the mapping download location and dropped zip extraction code. Please take a look again.

Copy link
Member

@miozune miozune left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

@Dream-Master Dream-Master merged commit 4466f68 into master Jun 3, 2023
1 check passed
@Dream-Master Dream-Master deleted the mclo.gs-support branch June 3, 2023 05:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants