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

GH-394 Add Sentry integration #808

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions buildSrc/src/main/kotlin/Versions.kt
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ object Versions {

const val SPOTIFY_COMPLETABLE_FUTURES = "0.3.6"

const val SENTRY = "8.0.0-alpha.2"

// tests
const val EXPRESSIBLE_JUNIT = "1.3.6"
const val GROOVY_ALL = "3.0.21"
Expand Down
4 changes: 4 additions & 0 deletions eternalcore-core/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -97,4 +97,8 @@ eternalShadow {
// caffeine
library("com.github.ben-manes.caffeine:caffeine:${Versions.CAFFEINE}")
libraryRelocate("com.github.benmanes.caffeine");

// sentry
library("io.sentry:sentry:${Versions.SENTRY}")
libraryRelocate("io.sentry")
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import com.eternalcode.core.publish.Publisher;
import com.eternalcode.core.publish.event.EternalInitializeEvent;
import com.eternalcode.core.publish.event.EternalShutdownEvent;
import io.sentry.Sentry;
import net.dzikoysk.cdn.entity.Contextual;
import org.bukkit.Server;
import org.bukkit.plugin.Plugin;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.eternalcode.core.bridge.litecommand;

import com.eternalcode.core.bridge.sentry.SentryLiteCommandExceptionHandler;
import com.eternalcode.core.injector.annotations.Bean;
import com.eternalcode.core.injector.annotations.component.BeanSetup;
import com.eternalcode.core.injector.bean.BeanFactory;
Expand Down Expand Up @@ -33,7 +34,8 @@ class LiteCommandsSetup implements Subscriber {
.commands(liteCommandsAnnotations)
.extension(new LiteAdventurePlatformExtension<CommandSender>(audiencesProvider), extension -> extension
.serializer(miniMessage)
);
)
.exception(Throwable.class, new SentryLiteCommandExceptionHandler());
}

@Bean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,12 @@
import dev.rollczi.litecommands.invocation.Invocation;
import dev.rollczi.litecommands.suggestion.SuggestionContext;
import dev.rollczi.litecommands.suggestion.SuggestionResult;
import org.bukkit.command.CommandSender;

import java.time.Duration;
import java.time.format.DateTimeParseException;
import java.util.Arrays;
import java.util.List;
import org.bukkit.command.CommandSender;

@LiteArgument(type = Duration.class)
class DurationArgument extends AbstractViewerArgument<Duration> {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package com.eternalcode.core.bridge.sentry;

import dev.rollczi.litecommands.handler.exception.ExceptionHandler;
import dev.rollczi.litecommands.handler.result.ResultHandlerChain;
import dev.rollczi.litecommands.invocation.Invocation;
import io.sentry.Sentry;
import org.bukkit.command.CommandSender;

public class SentryLiteCommandExceptionHandler implements ExceptionHandler<CommandSender, Throwable> {

@Override
public void handle(Invocation<CommandSender> invocation, Throwable t, ResultHandlerChain<CommandSender> resultHandlerChain) {
Sentry.captureException(t);
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance Sentry integration with additional context.

While the basic Sentry integration is correct, consider enriching the captured exceptions with additional context for more effective debugging and error analysis.

Here's a suggested improvement:

-Sentry.captureException(t);
+Sentry.withScope(scope -> {
+    scope.setExtra("commandName", invocation.label());
+    scope.setExtra("sender", invocation.sender().getName());
+    scope.setTag("commandType", invocation.commandType().getSimpleName());
+    Sentry.captureException(t);
+});

This change adds valuable context to Sentry events, including the command name, sender, and command type, which can greatly aid in debugging and error analysis.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Sentry.captureException(t);
Sentry.withScope(scope -> {
scope.setExtra("commandName", invocation.label());
scope.setExtra("sender", invocation.sender().getName());
scope.setTag("commandType", invocation.commandType().getSimpleName());
Sentry.captureException(t);
});

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
package com.eternalcode.core.bridge.sentry;

import com.eternalcode.core.configuration.implementation.PluginConfiguration;
import com.eternalcode.core.injector.DependencyInjectorException;
import com.eternalcode.core.injector.annotations.Inject;
import com.eternalcode.core.injector.annotations.component.BeanSetup;
import com.eternalcode.core.injector.bean.BeanException;
import com.eternalcode.core.publish.Subscribe;
import com.eternalcode.core.publish.Subscriber;
import com.eternalcode.core.publish.event.EternalInitializeEvent;
import com.google.common.collect.ImmutableList;
import dev.rollczi.litecommands.LiteCommandsException;
import io.papermc.lib.PaperLib;
import io.sentry.Sentry;
import org.bukkit.Server;
import org.bukkit.plugin.Plugin;
import org.xml.sax.SAXException;

import javax.xml.parsers.ParserConfigurationException;
import java.net.MalformedURLException;
import java.nio.file.FileSystemException;
import java.sql.SQLException;
import java.time.format.DateTimeParseException;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.ExecutionException;

@BeanSetup
class SentrySetup implements Subscriber {

private static final List<Class<? extends Throwable>> LOADER_OR_INJECTOR_EXCEPTIONS = List.of(
ReflectiveOperationException.class,
FileSystemException.class,
SAXException.class,
BeanException.class,
NoClassDefFoundError.class,
MalformedURLException.class,
ParserConfigurationException.class,
DependencyInjectorException.class
);

private static final List<Class<? extends Throwable>> DATABASE_EXCEPTIONS = ImmutableList.of(
SQLException.class,
ExecutionException.class
);

private static final List<Class<? extends Throwable>> LITECOMMANDS_EXCEPTIONS = ImmutableList.of(
LiteCommandsException.class,
DateTimeParseException.class
);


private final PluginConfiguration config;

@Inject
SentrySetup(PluginConfiguration config) {
this.config = config;
}

@Subscribe(EternalInitializeEvent.class)
public void onInitialize(Plugin plugin, Server server) {
if (this.config.sentryEnabled) {
Sentry.init(options -> {
options.setDsn("https://4bf366d0d9f1da00162b9e629a80be52@o4505014505177088.ingest.us.sentry.io/4506093905051648");
options.setTracesSampleRate(0.75);
options.setRelease(plugin.getDescription().getVersion());
options.setEnvironment(PaperLib.getEnvironment().getName());
options.setTag("plugins", Arrays.stream(server.getPluginManager().getPlugins()).toList().toString());
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid sending the list of plugins as a Sentry tag to protect privacy

Including the list of installed plugins may expose sensitive information about the server configuration or third-party plugins in use. This could have privacy implications or unintentionally reveal vulnerabilities.

Consider removing this tag or anonymizing the data to safeguard user privacy.


🛠️ Refactor suggestion

⚠️ Potential issue

Address potential compatibility issues with Java versions before 16

The toList() method in Arrays.stream(server.getPluginManager().getPlugins()).toList() is available from Java 16 onwards. If your project needs to support earlier Java versions, consider using collect(Collectors.toList()).

Improve formatting of the plugin list for Sentry tag

Using toString() on the list may not provide a clear and readable format in Sentry. To enhance readability, join the plugin names into a comma-separated string.

Apply this refactor:

-options.setTag("plugins", Arrays.stream(server.getPluginManager().getPlugins()).toList().toString());
+options.setTag("plugins", Arrays.stream(server.getPluginManager().getPlugins())
+    .map(Plugin::getName)
+    .collect(Collectors.joining(", ")));
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
options.setTag("plugins", Arrays.stream(server.getPluginManager().getPlugins()).toList().toString());
options.setTag("plugins", Arrays.stream(server.getPluginManager().getPlugins())
.map(Plugin::getName)
.collect(Collectors.joining(", ")));

options.setTag("serverVersion", server.getVersion());
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid sending the server version as a Sentry tag to enhance security

Exposing the server version may provide potential attackers with information about specific vulnerabilities associated with that version. It's advisable to omit this tag or generalize the version information to prevent disclosing detailed server metadata.

options.setBeforeSend((event, hint) -> {
Throwable throwable = event.getThrowable();

if (throwable != null) {
Copy link
Member

Choose a reason for hiding this comment

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

Trochę mało wydajne te streamy

Copy link
Member Author

Choose a reason for hiding this comment

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

Co proponujesz, zwykłe pętle for()?

Copy link
Member

Choose a reason for hiding this comment

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

Jakieś hashmapy sety coś takiego...

if (LOADER_OR_INJECTOR_EXCEPTIONS.stream().anyMatch(exception -> exception.isInstance(throwable))) {
event.setFingerprints(List.of("loader_or_injector"));
} else if (DATABASE_EXCEPTIONS.stream().anyMatch(exception -> exception.isInstance(throwable))) {
event.setFingerprints(List.of("database"));
} else if (LITECOMMANDS_EXCEPTIONS.stream().anyMatch(exception -> exception.isInstance(throwable))) {
event.setFingerprints(List.of("litecommands"));
}
}

return event;
});
});
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,13 @@
import com.eternalcode.core.feature.afk.AfkSettings;
import com.eternalcode.core.feature.automessage.AutoMessageSettings;
import com.eternalcode.core.feature.chat.ChatSettings;
import com.eternalcode.core.feature.helpop.HelpOpSettings;
import com.eternalcode.core.feature.jail.JailSettings;
import com.eternalcode.core.feature.randomteleport.RandomTeleportSettings;
import com.eternalcode.core.feature.randomteleport.RandomTeleportType;
import com.eternalcode.core.feature.helpop.HelpOpSettings;
import com.eternalcode.core.feature.spawn.SpawnSettings;
import com.eternalcode.core.injector.annotations.component.ConfigurationFile;
import com.eternalcode.core.feature.teleportrequest.TeleportRequestSettings;
import java.util.LinkedHashMap;
import java.util.Set;
import com.eternalcode.core.injector.annotations.component.ConfigurationFile;
import net.dzikoysk.cdn.entity.Contextual;
import net.dzikoysk.cdn.entity.Description;
import net.dzikoysk.cdn.entity.Exclude;
Expand All @@ -25,7 +23,9 @@

import java.io.File;
import java.time.Duration;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Set;

@ConfigurationFile
public class PluginConfiguration implements ReloadableConfig {
Expand All @@ -43,6 +43,10 @@ public class PluginConfiguration implements ReloadableConfig {
"#",
})

@Description({ "", "# Whether Sentry - the real-time error tracking system should be enabled",
"# We strongly recommend leaving this option on true, as it helps us fixing bugs faster."})
public boolean sentryEnabled = true;

@Description("# Whether the player should receive information about new plugin updates upon joining the server")
public boolean shouldReceivePluginUpdates = true;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import com.j256.ormlite.jdbc.DataSourceConnectionSource;
import com.j256.ormlite.support.ConnectionSource;
import com.zaxxer.hikari.HikariDataSource;
import io.sentry.Sentry;

import java.io.File;
import java.sql.SQLException;
Expand Down Expand Up @@ -95,6 +96,7 @@ public void close() {
this.connectionSource.close();
}
catch (Exception exception) {
Sentry.captureException(exception);
exception.printStackTrace();
}
}
Expand All @@ -112,6 +114,7 @@ public <T, ID> Dao<T, ID> getDao(Class<T> type) {
return (Dao<T, ID>) dao;
}
catch (SQLException exception) {
Sentry.captureException(exception);
throw new RuntimeException(exception);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
package com.eternalcode.core.database;

import com.eternalcode.core.configuration.implementation.PluginConfiguration;
import com.eternalcode.core.publish.Subscriber;
import com.eternalcode.core.publish.event.EternalShutdownEvent;
import com.eternalcode.core.injector.annotations.Bean;
import com.eternalcode.core.injector.annotations.component.BeanSetup;
import com.eternalcode.core.publish.Subscribe;
import com.eternalcode.core.publish.Subscriber;
import com.eternalcode.core.publish.event.EternalShutdownEvent;
import io.sentry.Sentry;

import java.io.File;
import java.sql.SQLException;
Expand All @@ -22,6 +23,7 @@ DatabaseManager databaseManager(PluginConfiguration pluginConfiguration, Logger
databaseManager.connect();
}
catch (SQLException exception) {
Sentry.captureException(exception);
logger.severe("Could not connect to database! Some functions may not work properly!");
throw new RuntimeException(exception);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.eternalcode.commons.scheduler.Scheduler;
import com.eternalcode.core.database.DatabaseManager;
import com.j256.ormlite.dao.Dao;
import io.sentry.Sentry;
import panda.std.function.ThrowingFunction;

import java.sql.SQLException;
Expand Down Expand Up @@ -62,6 +63,7 @@ protected <T, ID, R> CompletableFuture<R> action(Class<T> type, ThrowingFunction
completableFuture.complete(action.apply(dao));
}
catch (Throwable throwable) {
Sentry.captureException(throwable);
completableFuture.completeExceptionally(throwable);
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@
import com.eternalcode.core.configuration.implementation.PluginConfiguration;
import com.eternalcode.core.injector.annotations.Inject;
import com.eternalcode.core.injector.annotations.lite.LiteArgument;
import com.eternalcode.multification.notice.NoticeBroadcast;
import com.eternalcode.core.notice.NoticeService;
import com.eternalcode.core.translation.Translation;
import com.eternalcode.core.translation.TranslationManager;
import com.eternalcode.core.viewer.Viewer;
import com.eternalcode.core.viewer.ViewerService;
import com.eternalcode.multification.notice.NoticeBroadcast;
import dev.rollczi.litecommands.argument.Argument;
import dev.rollczi.litecommands.argument.parser.ParseResult;
import dev.rollczi.litecommands.invocation.Invocation;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import com.j256.ormlite.stmt.DeleteBuilder;
import com.j256.ormlite.table.DatabaseTable;
import com.j256.ormlite.table.TableUtils;
import io.sentry.Sentry;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;

Expand Down Expand Up @@ -53,6 +54,7 @@ public CompletableFuture<Boolean> isIgnored(UUID by, UUID target) {
return uuids.contains(target) || uuids.contains(IGNORE_ALL);
}
catch (ExecutionException exception) {
Sentry.captureException(exception);
throw new RuntimeException(exception);
}
});
Expand All @@ -70,6 +72,7 @@ public CompletableFuture<Void> ignore(UUID by, UUID target) {
}
}
catch (ExecutionException exception) {
Sentry.captureException(exception);
throw new RuntimeException(exception);
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.eternalcode.core.injector.annotations.Inject;
import com.eternalcode.core.injector.bean.BeanException;
import io.sentry.Sentry;

import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
Expand Down Expand Up @@ -47,9 +48,11 @@ public Object invokeMethod(Object instance, Method method, Object... additionalD
return method.invoke(instance, parameters);
}
catch (BeanException beanException) {
Sentry.captureException(beanException);
throw new DependencyInjectorException("Failed to invoke method " + method.getName() + " in " + declaringClass.getName() + "!", beanException, declaringClass);
}
catch (IllegalAccessException | IllegalArgumentException | InvocationTargetException exception) {
Sentry.captureException(exception);
throw new DependencyInjectorException(exception, declaringClass);
}
}
Expand Down Expand Up @@ -85,9 +88,11 @@ private <T> T newInstance(Constructor<T> constructor, Object... additionalDepend
return constructor.newInstance(parameters);
}
catch (BeanException beanException) {
Sentry.captureException(beanException);
throw new DependencyInjectorException("Failed to create a new instance of " + declaringClass.getName() + "!", beanException, declaringClass);
}
catch (InvocationTargetException | InstantiationException | IllegalAccessException exception) {
Sentry.captureException(exception);
throw new DependencyInjectorException(exception, declaringClass);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.eternalcode.core.injector.bean;

import io.sentry.Sentry;

import java.lang.reflect.Field;

public class LazyFieldBeanCandidate extends LazyBeanCandidate {
Expand All @@ -15,7 +17,10 @@ public LazyFieldBeanCandidate(Object instance, Field field) {
return field.get(instance);
}
catch (IllegalAccessException exception) {
throw new BeanException("Cannot access field " + field.getName() + " of " + instance.getClass().getName(), exception, field.getType());
String message = "Cannot access field " + field.getName() + " of " + instance.getClass().getName();
Sentry.captureException(exception);
Sentry.captureMessage(message);
throw new BeanException(message, exception, field.getType());
}
});
this.field = field;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.google.common.base.Preconditions;
import com.google.common.reflect.ClassPath;
import io.sentry.Sentry;

import java.io.IOException;
import java.util.ArrayList;
Expand Down Expand Up @@ -45,6 +46,7 @@ public static List<Class<?>> scanClasses(String packageToScan, ClassLoader class
return loadedClasses;
}
catch (IOException | ClassNotFoundException exception) {
Sentry.captureException(exception);
Copy link
Member

Choose a reason for hiding this comment

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

Trochę łopatologiczne Lucky jakoś globalnie kiedyś łapał błędy ale nw czy to tylko na wątki działa

Copy link
Member Author

Choose a reason for hiding this comment

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

poprawiłem indentację

Copy link
Member Author

Choose a reason for hiding this comment

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

Ale łopatologiczne jest używanie wszędzie Sentry#captureException, czy co masz na myśli? Musimy tak robić bo ja nie łapię wszystkich błędów celowo, np. tych argumentów z LC, bo przecież tam to jest wina stricte usera

Copy link
Member

Choose a reason for hiding this comment

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

Wątek w Javie ma coś takiego jak uncatched handler tyle, że musi wywalić wątek aby to złapało to część case już ogarnie... warto pomyśleć nad tym

throw new RuntimeException(exception);
}
}
Expand Down
2 changes: 2 additions & 0 deletions eternalcore-docs-api/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,6 @@ dependencies {
compileOnly("com.google.guava:guava:${Versions.GUAVA}")
compileOnly("com.google.code.gson:gson:${Versions.GSON}")
compileOnly("dev.rollczi:litecommands-framework:${Versions.LITE_COMMANDS}")

implementation("io.sentry:sentry:${Versions.SENTRY}")
}
Loading
Loading