nlthijs48 / errorsink Goto Github PK

Some performance statistics could be gathered to find bad regexes. Also tests could be done to check if we are on the main thread or not, so that we can avoid long work.

If an error occurs during shutdown it might not get reported, should test this and see why it does not report it. Maybe ErrorSink needs to slow down the shutdown a bit if an events is still sending to Sentry.io.

Currently regex parts as defined at the bottom of the config can be used in all regexes with for example {player}, but if multiple players need to be matched the regex will be invalid (having two named groups with the same name is forbidden). This could be solved by introducing something like {player|from}, to insert the player regex part, and expose it as a named group with the name from.

Just like AreaShop, add a maven argument to specify the build number and use that on the Jenkins build server to add the build number.

The breadcrumb appender is activated before the collector, therefore the message reported to Sentry contains itself in the breadcrumbs, leading to two of the same entries at the bottom of the breadcrumbs view.

Changing the order these appenders are added to the logger should fix the issue.

Causes a missing resource exception somewhere in Raven-Java, probably because the .jar file got overwritten by a new version.

Sentry also uses this order for stacktraces and the most recent one is normally the most relevant.

Also could add some custom statistics:

• Number of events.filters
• Number of events.rules
• Number of breadcrumbs.filters
• Number of breadcrumbs.rules

Scan all plugin jars for .class files and mark those packages as in app (and maybe all super packages of them?). Already using a set for package names so duplicates will be discarded. This scan might take too much time, so it should be async.

This will improve this issue because the com.intellectualcrafters.plot... package would also get marked as in-app.

It looks like log4j2 has been updated and the workarounds in BukkitSentryAppender for the old version of log4j are not required anymore (getLogMillis() and log level comparisons). It is probably possible to work around this issue by moving the workaround to a class that extends BukkitSentryAppender and using that only if the version is lower than 1.12.

A bunch of classes do not depend on Bukkit and can directly be used, but some need adjustment and the startup class + bungee.yml file needs to be created.

A recent change in Raven-Java changes the list that marks stackframes as in-app from a blacklist to a whitelist. For our purposes we would like all frames in plugins to be in-app, maybe this could be done by getting the main class location for each plugin and marking that package as in-app. If that does not work we can also mark everything as in-app.

Implementation:
Extend DefaultRavenFactory and override getInAppFrames(). Probably return something simple at first, and update at the first server tick with a list of packages used by plugins. Could also return a list of the alphabet at the start to mark everything as in app.

Might need to add an appender another root logger, not sure how this happens

Implementation:
Add InteractiveMessenger dependency (for messages and translation), setup command, call onDisable() and onEnable() to reload (these are already set up with reloading in mind).

This ensures more startup and shutdown problem are caught. Might want to simply do nothing onDisable(), so that messages after disable are still picked up.

It would be nice if commands executed from the console and the replies they get back could be recorded. Might not be easy to do technically.

Implementation:
Get commands send from console here: https://hub.spigotmc.org/javadocs/spigot/org/bukkit/event/server/ServerCommandEvent.html

Add an update checker that uses Github or Spigot

They changed the name and there is a new version available.

Add an option to prevent events getting into the log file and console. This helps to keep these clean and relates to the functionality of ErrorSink.

Suggested by ChillerKraft

Test and fix collection of FATAL messages.

Noted by ChillerKraft

This looks like a fantastic project!

If you're interested, I'd like to work on adding Sponge support to this plugin.

Sentry already has burst limits to prevent spam, but this will still burn through the 10k limit per month of the free plan very quickly. So client side rate limiting could fix this problem.

Allows to test if the plugin works, can also be used in the setup tutorial.

Currently all exceptions in events are shown as EventException, which is not helpful at all. The message defined in these events is already usable, like Could not pass event InventoryClickEvent to MysteryBox v1.4.

Implementation:
It looks like Sentry prefers using something coming from the exception, so modifying ExceptionInterface should do the trick.

There are some breaking changes to breadcrumbs internally, but the upgrade should not be hard. Can also consider using the new SingletonContextManager for things like plugin versions (how to take into account things like PlugMan though?).

This allows 'contains' checks by simply giving a string and full message matches using the start of the line mark ^ and end of the line mark $

For example exceptions that are printed to System.out.err are redirected from the standard stream to the logging framework (as level=warn for some reason), which they will go through line-by-line. This causes a lot of events to Sentry, where none of them shows the complete picture.

• Maybe could be solved by changing the System.out.err stream ourselves, where these kind of exceptions might still be 1 thing. In this case we need to be careful to not cause any side effects.
• DedicatedServer.java wraps the streams:

  System.setOut(new PrintStream(new LoggerOutputStream(logger, Level.INFO), true));
  System.setErr(new PrintStream(new LoggerOutputStream(logger, Level.WARN), true));

• Because the second argument of PrintStream is true it will automatically flush, meaning every \n that is printed will send a line to the LoggerOutputStream, causing the line-by-line logging.
• Could probably call System.setOut() again and use a modified PrintStream that does not flush on \n, but still flushes after write() calls.

Spigot server crash output has the same problem, it prints all running threads to the console, which causes Sentry to send an event for each line.

• These crash reports are logged by the Spigot Watchdog Thread, so that might be used to detect them. Either to drop them or to combine all lines of them and put it in one event (this kind of thing could be made more general and usable for other things probably).