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

Rewrite crash reporting #1820

Merged
merged 1 commit into from
Jul 11, 2022

Conversation

nielsvanvelzen
Copy link
Member

This PR replaces our current crash reporting with a new implementation that sends the logs to the last used Jellyfin server. The reports use markdown with a YAML header indicating the format. This can be used in the future if we create a log viewer in jellyfin-web.

The crash reporting functionality caches the Jellyfin server url and access token when restoring a session (login/app open). When the app crashes these values are used to manually connect to the Jellyfin server instead of using the SDK. This is so we can catch SDK issues too.

Changes

  • Fix ACRA process running Application.onCreate
  • Move crash reporting to new TelemetryService
  • Use ACRA Toast instead of dialog (always auto-submits now)
    • This is to avoid an issue where the dialog is inresponsive
  • Upload crash report to Jellyfin server instead of Tracepot
  • Remove ACRA limitter

Issues

}

class AcraPluginLoader(vararg plugins: Class<out Plugin>) : PluginLoader {
private val simplePluginLoader = SimplePluginLoader(*plugins)

Check notice

Code scanning

In most cases using a spread operator causes a full copy of the array to be created before calling a method. This may result in a performance penalty.

In most cases using a spread operator causes a full copy of the array to be created before calling a method. This may result in a performance penalty.
@nielsvanvelzen
Copy link
Member Author

Right now 2 preferences are removed:

  • pref_acra_alwaysaccept - This is now the (only) default option, the dialog is removed
  • pref_acra_syslog - System logs are now always included, I'm not sure if we want to keep this option

Additionally, because the options moved to a different preference store, the value is reset and reporting is now enabled for everyone when updating.

@nielsvanvelzen
Copy link
Member Author

Additional changes pushed:

  • Add back option to enable/disable logs (previously called system logs), enabled by default
    • The description previously stated that this could include logs from other applications, this is not true so the naming has changed to not mention "system logs" but just "logs" instead.
  • Rename the strings related to crash reports, they don't use "acra" in the name anymore and this forces all translations to be rewritten.

@nielsvanvelzen nielsvanvelzen added the release-highlight Pull request might be suitable for mentioning in the release blog post label Jul 7, 2022
@jellyfin-bot jellyfin-bot added the merge conflict Conflicts prevent merging label Jul 10, 2022
@jellyfin-bot jellyfin-bot removed the merge conflict Conflicts prevent merging label Jul 10, 2022
@nielsvanvelzen nielsvanvelzen merged commit 744b28d into jellyfin:master Jul 11, 2022
@nielsvanvelzen nielsvanvelzen deleted the server-crash-report branch July 11, 2022 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-highlight Pull request might be suitable for mentioning in the release blog post
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants