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

Added SMAPI crash diagnostic #2699

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

Conversation

Pickysaurus
Copy link
Contributor

@Pickysaurus Pickysaurus commented Feb 21, 2025

Added a new diagnostic for Stardew Valley which will show a Critical Health Check warning if the last log generated by SMAPI is a crash log.

It works by:

  1. Checking the %appdata%\StardewValley\ErrorLogs (or Linux equivalent (needs testing!))
  2. Compares the last edit time of all log files.
  3. A diagnostic is raised if the latest log file is SMAPI-crash.txt and it was last edited less than 12 hours ago.
  4. When the latest log file is SMAPI-log.txt the diagnostic will not show.

Possible improvements:

  • Detect the last time we launched the game (if possible?) and use that to work out how relevant the log is
  • Localise the date/time based on the users app locale
  • Add tests

Added a new diagnostic for Stardew Valley which will show a Critical Health Check warning if the last log generated by SMAPI is a crash log.

It works by:
1. Checking the `C:\Users\MikeW\AppData\Roaming\StardewValley\ErrorLogs` (or Linux equivalnet (needs testing!)
2. Compares the last edit time of all log files.
3. If the latest log file is SMAPI-crash.txt, a diagnostic is raised.
Added a 12 hour tolerance on the diagnostic.
Added a new LogFilePathWithEditTime class to avoid re-fetching the same data.
Moved the method to find the last log file to Helpers.cs
Moved the logs folder location, log names and log upload url to Constants.cs
Cleaned up SMAPILogDiangnosticEmitter.cs to use the new locations for the data.
@Pickysaurus Pickysaurus added the Epic: Diagnostics This is related to the Diagnostic System label Feb 24, 2025
@erri120
Copy link
Member

erri120 commented Feb 24, 2025

I'm not sure what prompted this PR, whether this is just a pet project or something else. I think the only open issue related to SMAPI logs is #1721 which is about sharing logs. We had talks about a log viewer and parser before, but that got never scheduled or even written down.

Regardless of the ticket state, log parsing and diagnostics should be two separate systems. The idea behind diagnostics is to provide static analysis of the loadout and notify the user about every issue we can identify statically before they launch the game. Logs are only produced after launching the game. In the ideal world, we'd catch every issue with diagnostics before they start the game, not notify them afterwards using log parsing.

Log parsing is still helpful, especially for mod creators, and for debugging issues that can't be statically analyzed like runtime conflicts, which is why a separate system that is more fleshed out and specialized for logs would be needed. Probably a new left menu entry that opens a log viewer with a parser, a share button and many other gadgets to help work with logs. There are just too many features that we'd want in regards to logs that the diagnostic system doesn't provide because it was built for something else entirely.

I don't know what you want to do with this PR, probably fine to keep it as a draft PR for now until we figure out what we want to do for logs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Epic: Diagnostics This is related to the Diagnostic System
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants