Skip to content

Commit

Permalink
Add Sentry Error Tracking (MCCTeam#2670)
Browse files Browse the repository at this point in the history
* Add Sentry Error Tracking

* Omit personally identifiable information and add additional sentry context

* Remove debug message

* Make sentry opt-out and add related notices and strings

Also add Minecraft Version to error context

* Update build to send release info to sentry

* Adjust sentry error tracking

- Send the user-friendly Minecraft Version in the error logs
- Capture exceptions in more parts of the application

We now capture exceptions from the following locations:
- Protocol18 (1.8+) Packet errors
- Errors during client initialization phase (When client is about to start, session keys are NEVER sent to sentry)

* Make Sentry DSN configurable and repository-specific

The Sentry DSN will automatically be filled out on the main repository through the Github Actions build.

* Update build-and-release.yml

Update sed command

* style: change variable name

nitpick, just to make it a little bit more descriptive

* Add Sentry branding in README.

* remove old code (merge conflict)
  • Loading branch information
breadbyte authored Jun 21, 2024
1 parent 8756ff5 commit 0855109
Show file tree
Hide file tree
Showing 11 changed files with 148 additions and 24 deletions.
14 changes: 13 additions & 1 deletion .github/workflows/build-and-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ jobs:
run: |
echo '' >> ${{ env.assembly-info }}
echo "[assembly: AssemblyConfiguration(\"GitHub build ${{ github.run_number }}, built on ${{ env.date_dashed }} from commit ${{ env.commit }}\")]" >> ${{ env.assembly-info }}
sed -i -e 's|SentryDSN = "";|SentryDSN = "${{ secrets.SENTRY_DSN }}";|g' ${{ env.project-path }}/Program.cs
- name: Build Target
run: dotnet publish ${{ env.project-path }}.sln -f ${{ env.target-version }} -r ${{ matrix.target }} ${{ env.compile-flags }}
Expand Down Expand Up @@ -177,6 +178,7 @@ jobs:
run: |
echo '' >> ${{ env.assembly-info }}
echo "[assembly: AssemblyConfiguration(\"GitHub build ${{ github.run_number }}, built on ${{ env.date_dashed }} from commit ${{ env.commit }}\")]" >> ${{ env.assembly-info }}
sed -i -e 's|SentryDSN = "";|SentryDSN = "${{ secrets.SENTRY_DSN }}";|g' ${{ env.project-path }}/Program.cs
- name: Build Target
run: dotnet publish ${{ env.project-path }}.sln -f ${{ env.target-version }} -r ${{ matrix.target }} ${{ env.compile-flags }}
Expand All @@ -195,7 +197,17 @@ jobs:
filePath: ${{ env.target-out-path }}/mcc-${{ matrix.target }}.zip
assetName: ${{ env.PROJECT }}-${{ (contains(matrix.target, 'linux-x64') && 'linux.zip') || (contains(matrix.target, 'win-x86') && 'windows-x86.zip') || (contains(matrix.target, 'win-x64') && 'windows-x64.zip') || (contains(matrix.target, 'linux-arm64') && 'linux-arm64.zip') || (contains(matrix.target, 'osx-x64') && 'osx.zip') }}
tag: ${{ format('{0}-{1}', env.date, github.run_number) }}


- name: Sentry Release
uses: getsentry/[email protected]
env:
SENTRY_AUTH_TOKEN: ${{ secrets.SENTRY_AUTH_TOKEN }}
SENTRY_ORG: ${{ secrets.SENTRY_ORG }}
SENTRY_PROJECT: ${{ secrets.SENTRY_PROJECT }}
with:
environment: production
dist: ${{ format('{0}-{1}', env.date, github.run_number) }}

determine-build:
runs-on: ubuntu-latest
strategy:
Expand Down
30 changes: 30 additions & 0 deletions MinecraftClient/McClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
using MinecraftClient.Protocol.Session;
using MinecraftClient.Proxy;
using MinecraftClient.Scripting;
using Sentry;
using static MinecraftClient.Settings;

namespace MinecraftClient
Expand Down Expand Up @@ -191,6 +192,33 @@ public McClient(SessionToken session, PlayerKeyPair? playerKeyPair, string serve
Log.WarnEnabled = Config.Logging.WarningMessages;
Log.ErrorEnabled = Config.Logging.ErrorMessages;

// SENTRY: Send our client version and server version to Sentry
SentrySdk.ConfigureScope(scope =>
{
scope.SetTag("Protocol Version", protocolversion.ToString());
scope.SetTag("Minecraft Version", ProtocolHandler.ProtocolVersion2MCVer(protocolversion));
scope.SetTag("MCC Build", Program.BuildInfo == null ? "Debug" : Program.BuildInfo);

if (forgeInfo != null)
scope.SetTag("Forge Version", forgeInfo?.Version.ToString());

scope.Contexts["Server Information"] = new
{
ProtocolVersion = protocolversion,
MinecraftVersion = ProtocolHandler.ProtocolVersion2MCVer(protocolversion),
ForgeInfo = forgeInfo?.Version
};

scope.Contexts["Client Configuration"] = new
{
TerrainAndMovementsEnabled = terrainAndMovementsEnabled,
InventoryHandlingEnabled = inventoryHandlingEnabled,
EntityHandlingEnabled = entityHandlingEnabled
};
});

SentrySdk.StartSession();

/* Load commands from Commands namespace */
LoadCommands();

Expand Down Expand Up @@ -588,6 +616,8 @@ public void OnConnectionLost(ChatBot.DisconnectReason reason, string message)
}
}

SentrySdk.EndSession();

if (!will_restart)
{
ConsoleInteractive.ConsoleReader.StopReadThread();
Expand Down
1 change: 1 addition & 0 deletions MinecraftClient/MinecraftClient.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
<PackageReference Include="Microsoft.CSharp" Version="4.7.0" />
<PackageReference Include="Microsoft.Windows.Compatibility" Version="7.0.3" />
<PackageReference Include="Samboy063.Tomlet" Version="5.0.1" />
<PackageReference Include="Sentry" Version="4.1.0" />
<PackageReference Include="SingleFileExtractor.Core" Version="1.0.1" />
<PackageReference Include="starksoft.aspen" Version="1.1.8">
<NoWarn>NU1701</NoWarn>
Expand Down
30 changes: 30 additions & 0 deletions MinecraftClient/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
using MinecraftClient.Protocol.Session;
using MinecraftClient.Scripting;
using MinecraftClient.WinAPI;
using Sentry;
using Tomlet;
using static MinecraftClient.Settings;
using static MinecraftClient.Settings.ConsoleConfigHealper.ConsoleConfig;
Expand Down Expand Up @@ -50,14 +51,31 @@ static class Program
public static readonly string? BuildInfo = null;

private static Tuple<Thread, CancellationTokenSource>? offlinePrompt = null;
private static IDisposable _sentrySdk;
private static bool useMcVersionOnce = false;
private static string settingsIniPath = "MinecraftClient.ini";

// [SENTRY]
// Setting this string to an empty string will disable Sentry
private const string SentryDSN = "";

/// <summary>
/// The main entry point of Minecraft Console Client
/// </summary>
static void Main(string[] args)
{
// [SENTRY] Initialize Sentry SDK only if the DSN is not empty
if (SentryDSN != string.Empty) {
_sentrySdk = SentrySdk.Init(options =>
{
options.Dsn = SentryDSN;
options.AutoSessionTracking = true;
options.IsGlobalModeEnabled = true;
options.EnableTracing = true;
options.SendDefaultPii = false;
});
}

Task.Run(() =>
{
// "ToLower" require "CultureInfo" to be initialized on first run, which can take a lot of time.
Expand Down Expand Up @@ -139,6 +157,12 @@ static void Main(string[] args)
if (newlyGenerated)
ConsoleIO.WriteLineFormatted("§c" + Translations.mcc_settings_generated);
ConsoleIO.WriteLine(Translations.mcc_run_with_default_settings);

// Only show the Sentry message if the DSN is not empty
// as Sentry will not be initialized if the DSN is empty
if (SentryDSN != string.Empty) {
ConsoleIO.WriteLine(Translations.mcc_sentry_logging);
}
}
else if (!loadSucceed)
{
Expand Down Expand Up @@ -182,6 +206,9 @@ static void Main(string[] args)
ConsoleIO.WriteLine(string.Format(Translations.mcc_help_us_translate, Settings.TranslationProjectUrl));
WriteBackSettings(true); // format
}

if (!Config.Main.Advanced.EnableSentry)
_sentrySdk.Dispose();
}

//Other command-line arguments
Expand Down Expand Up @@ -632,6 +659,9 @@ private static void InitializeClient()
}
catch (Exception e)
{
// [SENTRY]
SentrySdk.CaptureException(e);

ConsoleIO.WriteLine(e.Message);
ConsoleIO.WriteLine(e.StackTrace ?? "");
HandleFailure(); // Other error
Expand Down
26 changes: 23 additions & 3 deletions MinecraftClient/Protocol/Handlers/Protocol18.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
using MinecraftClient.Protocol.Session;
using MinecraftClient.Proxy;
using MinecraftClient.Scripting;
using Sentry;
using static MinecraftClient.Settings;
using static MinecraftClient.Settings.MainConfigHelper.MainConfig.GeneralConfig;

Expand Down Expand Up @@ -370,6 +371,10 @@ internal Tuple<int, Queue<byte>> ReadNextPacket()
/// <returns>TRUE if the packet was processed, FALSE if ignored or unknown</returns>
internal bool HandlePacket(int packetId, Queue<byte> packetData)
{
// This copy is necessary because by the time we get to the catch block,
// the packetData queue will have been processed and the data will be lost
var _copy = packetData.ToArray();

try
{
switch (currentState)
Expand Down Expand Up @@ -430,7 +435,7 @@ internal bool HandlePacket(int packetId, Queue<byte> packetData)
World.StoreDimensionList(registryCodec);

break;

case ConfigurationPacketTypesIn.RemoveResourcePack:
if (dataTypes.ReadNextBool(packetData)) // Has UUID
dataTypes.ReadNextUUID(packetData); // UUID
Expand Down Expand Up @@ -461,14 +466,29 @@ internal bool HandlePacket(int packetId, Queue<byte> packetData)
innerException.InnerException is SocketException)
throw; //Thread abort or Connection lost rather than invalid data

throw new System.IO.InvalidDataException(
var exception = new System.IO.InvalidDataException(
string.Format(Translations.exception_packet_process,
packetPalette.GetIncomingTypeById(packetId),
packetId,
protocolVersion,
currentState == CurrentState.Login,
innerException.GetType()),
innerException);

SentrySdk.AddBreadcrumb(new Breadcrumb("S -> C Packet", "network", new Dictionary<string, string>()
{
{ "Packet ID", packetId.ToString() },
{ "Packet Type ", packetPalette.GetIncomingTypeById(packetId).ToString() },
{ "Protocol Version", protocolVersion.ToString() },
{ "Minecraft Version", ProtocolHandler.ProtocolVersion2MCVer(protocolVersion) },
{ "Current State", currentState.ToString() },
{ "Packet Data", string.Join(" ", _copy.Select(b => b.ToString("X2"))) },
{ "Inner Exception", innerException.GetType().ToString() }
}, "packet", BreadcrumbLevel.Error));

SentrySdk.CaptureException(exception);

throw exception;
}

return true;
Expand Down Expand Up @@ -4561,4 +4581,4 @@ internal enum CurrentState
Configuration,
Play
}
}
}
Loading

0 comments on commit 0855109

Please sign in to comment.