Better syntax error handling (#332)

* Debugging: Add tasks and launch

#!components: grid-bot,grid-bot-recovery
#!deployable-components: grid-bot

~ Add build tasks for debug and full debug builds.
~ Add launch scripts for local debugging.

* Projects: Bump Discord.Net

Bumps Discord.Net to 3.15.3

* Update to OnLogMessage

Support logging GatewayReconnectException, WebSocketException, WebSocketClosedException, and TaskCanceledException in debug builds

* Feature: Script logging

Implement logging of scripts to a Discord channel, helps with future debugging of issues that arrise.
Has duplication detection via hash checking of script contents.

* LuaVM

Suit changes made to parsing system

* ExecuteScript: Syntax error and script logging

~ Log scripts just before preprocessing.
~ Add detection of grid-server level syntax errors.
~ Add detection of invalid json return data.
~ Add detection of invalid ASCII return data.

* Update Runner.cs

~ Add script logger to dependency injection container.

* Update build.yml

Signed-off-by: Nikita Petko <petko@vmminfra.net>

* Update deploy.yml

Signed-off-by: Nikita Petko <petko@vmminfra.net>

* ScriptLogger: Persistence

~ Do not persist script hashes immediately, as this could cause a major influx of Vault writes.

* Update deploy.yml

Signed-off-by: Nikita Petko <petko@vmminfra.net>

* Update .component.yaml

Signed-off-by: Nikita Petko <petko@vmminfra.net>

* Fix for hashes

---------

Signed-off-by: Nikita Petko <petko@vmminfra.net>

Author: nikita-petko

.github/workflows/build.yml

@@ -463,12 +463,10 @@ jobs:
             let deployableComponentsMap = '';
 
             for (const component of components) {
-              let [name, version] = component.split(':');
-
-              if (!version) version = 'latest';
+              const [name] = component.split(':');
 
               if (deployableComponents.includes(name)) {
-                deployableComponentsMap += `${component}:${version},`;
+                deployableComponentsMap += `${component},`;
               }
             }
 

.github/workflows/deploy.yml

@@ -47,7 +47,7 @@ jobs:
           component-search-directories: services
 
       - name: Components to Nomad Jobs
-        uses: mfdlabs/component-nomad-parser-action@v11
+        uses: mfdlabs/component-nomad-parser-action@v14
         id: components-to-nomad-jobs
         env:
           NOMAD_ENVIRONMENT: ${{ github.event.inputs.nomad_environment }}

.vscode/launch.json

@@ -0,0 +1,20 @@
+{
+    "version": "0.2.0",
+    "configurations": [
+        {
+            "name": "Grid.Bot",
+            "type": "coreclr",
+            "request": "launch",
+            "preLaunchTask": "build",
+            "program": "${workspaceFolder}/services/grid-bot/src/bin/Debug/net8.0/Grid.Bot.dll",
+            "cwd": "${workspaceFolder}/services/grid-bot/src/",
+            "console": "internalConsole",
+            "stopAtEntry": false,
+            "logging": {
+                "moduleLoad": false
+            },
+            "requireExactSource": false,
+            "envFile": "${workspaceFolder}/.env"
+        }
+    ]
+}

.vscode/tasks.json

@@ -0,0 +1,34 @@
+{
+    "version": "2.0.0",
+    "tasks": [
+        {
+            "label": "build",
+            "command": "dotnet",
+            "type": "process",
+            "args": [
+                "build",
+                "${workspaceFolder}/services/grid-bot/src/Grid.Bot.csproj",
+                "/property:GenerateFullPaths=true",
+                "/consoleloggerparameters:NoSummary",
+                "/p:Configuration=Debug",
+                "/p:Platform=AnyCPU",
+            ],
+            "problemMatcher": "$msCompile"
+        },
+        {
+            "label": "build-full",
+            "command": "dotnet",
+            "type": "process",
+            "args": [
+                "build",
+                "${workspaceFolder}/services/grid-bot/src/Grid.Bot.csproj",
+                "/property:GenerateFullPaths=true",
+                "/consoleloggerparameters:NoSummary",
+                "/p:Configuration=Debug",
+                "/p:Platform=AnyCPU",
+                "/p:LocalBuild=true"
+            ],
+            "problemMatcher": "$msCompile"
+        }
+    ]
+}

services/grid-bot/.component.yaml

@@ -18,6 +18,7 @@ build:
 
 deployment:
   count: 1
+  namespace: grid-bot
 
   job: grid-bot-${{ env.NOMAD_SHORT_ENVIRONMENT }}
 

services/grid-bot/lib/commands/Modules/ExecuteScript.cs

@@ -2,6 +2,7 @@ namespace Grid.Bot.Interactions.Public;
 
 using System;
 using System.IO;
+using System.Xml;
 using System.Linq;
 using System.Text;
 using System.Diagnostics;
@@ -41,6 +42,7 @@ namespace Grid.Bot.Interactions.Public;
 /// <param name="jobManager">The <see cref="IJobManager"/>.</param>
 /// <param name="adminUtility">The <see cref="IAdminUtility"/>.</param>
 /// <param name="discordWebhookAlertManager">The <see cref="IDiscordWebhookAlertManager"/>.</param>
+/// <param name="scriptLogger">The <see cref="IScriptLogger"/>.</param>
 /// <param name="gridServerFileHelper">The <see cref="IGridServerFileHelper"/>.</param>
 /// <exception cref="ArgumentNullException">
 /// - <paramref name="logger"/> cannot be null.
@@ -52,6 +54,7 @@ namespace Grid.Bot.Interactions.Public;
 /// - <paramref name="jobManager"/> cannot be null.
 /// - <paramref name="adminUtility"/> cannot be null.
 /// - <paramref name="discordWebhookAlertManager"/> cannot be null.
+/// - <paramref name="scriptLogger"/> cannot be null.
 /// - <paramref name="gridServerFileHelper"/> cannot be null.
 /// </exception>
 [Group("execute", "Commands used for executing Luau code.")]
@@ -65,6 +68,7 @@ public partial class ExecuteScript(
     IJobManager jobManager,
     IAdminUtility adminUtility,
     IDiscordWebhookAlertManager discordWebhookAlertManager,
+    IScriptLogger scriptLogger,
     IGridServerFileHelper gridServerFileHelper
 ) : InteractionModuleBase<ShardedInteractionContext>
 {
@@ -83,13 +87,18 @@ IGridServerFileHelper gridServerFileHelper
     private readonly IJobManager _jobManager = jobManager ?? throw new ArgumentNullException(nameof(jobManager));
     private readonly IAdminUtility _adminUtility = adminUtility ?? throw new ArgumentNullException(nameof(adminUtility));
     private readonly IDiscordWebhookAlertManager _discordWebhookAlertManager = discordWebhookAlertManager ?? throw new ArgumentNullException(nameof(discordWebhookAlertManager));
+    private readonly IScriptLogger _scriptLogger = scriptLogger ?? throw new ArgumentNullException(nameof(scriptLogger));
     private readonly IGridServerFileHelper _gridServerFileHelper = gridServerFileHelper ?? throw new ArgumentNullException(nameof(gridServerFileHelper));
 
 
-    [GeneratedRegex(@"```(.*?)\s(.*?)```", RegexOptions.IgnoreCase | RegexOptions.Singleline, "en-GB")]
+    [GeneratedRegex(@"```(.*?)\s(.*?)```", RegexOptions.IgnoreCase | RegexOptions.Singleline)]
     private static partial Regex CodeBlockRegex();
-    [GeneratedRegex("[\"“‘”]", RegexOptions.IgnoreCase | RegexOptions.Compiled, "en-GB")]
+    [GeneratedRegex("[\"“‘”]", RegexOptions.IgnoreCase | RegexOptions.Compiled)]
     private static partial Regex QuotesRegex();
+    [GeneratedRegex(@"Execute Script:(\d+): (.+)", RegexOptions.IgnoreCase | RegexOptions.Compiled)]
+    private static partial Regex GridSyntaxErrorRegex();
+
+    private const string _ErrorConvertingToJson = "Can't convert to JSON";
 
     /// <inheritdoc cref="InteractionModuleBase{TContext}.BeforeExecuteAsync(ICommandInfo)"/>
     public override async Task BeforeExecuteAsync(ICommandInfo command)
@@ -164,6 +173,9 @@ private static string GetCodeBlockContents(string s)
         return (input, null);
     }
 
+    private async Task LuaErrorAsync(string error)
+        => await HandleResponseAsync(null, new() { ErrorMessage = error, ExecutionTime = 0, Success = false });
+
     private async Task HandleResponseAsync(string result, ReturnMetadata metadata)
     {
         var builder = new EmbedBuilder()
@@ -284,6 +296,8 @@ string script
 
         var originalScript = script;
 
+        await _scriptLogger.LogScriptAsync(script, Context);
+
         if (ContainsUnicode(script))
         {
             await FollowupAsync("The script cannot contain unicode characters as grid-servers cannot support unicode in transit.");
@@ -360,12 +374,47 @@ string script
 
             Task.Run(() => _jobManager.CloseJob(job, false));
 
-            await AlertForSystem(script, originalScript, scriptId, scriptName, ex);
-
             if (ex is FaultException)
             {
-                // Needs to be reported, get the original script, the fully constructed script and all information about channels, users, etc.
-                await FollowupAsync("There was an internal error, please try again later.");
+                var message = ex.Message;
+                if (GridSyntaxErrorRegex().IsMatch(message))
+                {
+                    var match = GridSyntaxErrorRegex().Match(message);
+                    var line = match.Groups[1].Value;
+                    var error = match.Groups[2].Value;
+
+                    // We need to subtract the lines that the template adds (otherwise for one liners it will appear to be on line like 500 and something)
+                    if (_scriptsSettings.LuaVMEnabled)
+                    {
+                        const string _marker = "{0}";
+
+                        var template = _luaUtility.LuaVMTemplate;
+                        var templateLines = template.Split('\n');
+
+                        var lineIndex = Array.FindIndex(templateLines, line => line.StartsWith(_marker));
+
+                        if (lineIndex != -1)
+                            line = (int.Parse(line) - lineIndex).ToString();
+                    }
+
+                    await LuaErrorAsync($"Line {line}: {error}");
+
+                    return;
+                }
+
+                if (message.Contains(_ErrorConvertingToJson))
+                {
+                    await LuaErrorAsync("The script returned a value that could not be converted to JSON.");
+
+                    return;
+                }
+            }
+
+            // If ex.InnerException.InnerException is a XmlException, it's likely that the script returned invalid ASCII characters.
+            // Catch this and alert the user (only in the case of ex is CommunicationException, ex.InnerException is InvalidOperationException and ex.InnerException.InnerException is XmlException)
+            if (ex is CommunicationException && ex.InnerException is InvalidOperationException && ex.InnerException.InnerException is XmlException)
+            {
+                await LuaErrorAsync("The script returned invalid ASCII characters.");
 
                 return;
             }
@@ -377,6 +426,8 @@ string script
                 return;
             }
 
+            await AlertForSystem(script, originalScript, scriptId, scriptName, ex);
+
             throw;
         }
         finally

services/grid-bot/lib/commands/Shared.Commands.csproj

@@ -32,9 +32,9 @@
   </ItemGroup>
 
   <ItemGroup>
-    <PackageReference Include="Discord.Net.WebSocket" Version="3.15.2" />
-    <PackageReference Include="Discord.Net.Commands" Version="3.15.2" />
-    <PackageReference Include="Discord.Net.Interactions" Version="3.15.2" />
+    <PackageReference Include="Discord.Net.WebSocket" Version="3.15.3" />
+    <PackageReference Include="Discord.Net.Commands" Version="3.15.3" />
+    <PackageReference Include="Discord.Net.Interactions" Version="3.15.3" />
     <PackageReference Include="Loretta.CodeAnalysis.Lua" Version="0.2.11" />
     <PackageReference Include="Microsoft.CodeAnalysis.CSharp.Scripting" Version="3.11.0" />
     <PackageReference Include="Microsoft.CodeAnalysis.Scripting.Common" Version="3.11.0" />

services/grid-bot/lib/events/Events/OnLogMessage.cs

@@ -93,6 +93,7 @@ public Task Invoke(LogMessage message)
         if (message.Exception != null)
         {
 #if DEBUG || DEBUG_LOGGING_IN_PROD
+#if !DEBUG // Don't log these exceptions outside of debug mode.
             if (message.Exception is GatewayReconnectException)
                 return Task.CompletedTask;
 
@@ -106,6 +107,7 @@ public Task Invoke(LogMessage message)
             if (message.Exception is TaskCanceledException &&
                 !_settings.DebugAllowTaskCanceledExceptions)
                 return Task.CompletedTask;
+#endif
 
             _logger.Error(
                 "Source = {0}, Message = {1}, Exception = {2}",

services/grid-bot/lib/events/Shared.Events.csproj

@@ -10,9 +10,9 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <PackageReference Include="Discord.Net.WebSocket" Version="3.15.2" />
-    <PackageReference Include="Discord.Net.Commands" Version="3.15.2" />
-    <PackageReference Include="Discord.Net.Interactions" Version="3.15.2" />
+    <PackageReference Include="Discord.Net.WebSocket" Version="3.15.3" />
+    <PackageReference Include="Discord.Net.Commands" Version="3.15.3" />
+    <PackageReference Include="Discord.Net.Interactions" Version="3.15.3" />
 
     <PackageReference Include="prometheus-net" Version="8.0.1" />
   </ItemGroup>

services/grid-bot/lib/settings/Providers/ScriptsSettings.cs

@@ -1,5 +1,7 @@
 namespace Grid.Bot;
 
+using System;
+
 /// <summary>
 /// Settings provider for all script execution related stuff.
 /// </summary>
@@ -31,4 +33,40 @@ public class ScriptsSettings : BaseSettingsProvider
         nameof(LuaVMEnabled),
         true
     );
+
+    /// <summary>
+    /// Gets the percentage to use for logging scripts.
+    /// </summary>
+    public int ScriptLoggingPercentage => GetOrDefault(
+        nameof(ScriptLoggingPercentage),
+        0
+    );
+
+    /// <summary>
+    /// Gets a Discord webhook URL to send script logs to.
+    /// </summary>
+    public string ScriptLoggingDiscordWebhookUrl => GetOrDefault(
+        nameof(ScriptLoggingDiscordWebhookUrl),
+        string.Empty
+    );
+
+    /// <summary>
+    /// Gets or sets a list of hashes of scripts that have already been logged and should not be logged again.
+    /// </summary>
+    public string[] LoggedScriptHashes
+    {
+        get => GetOrDefault(
+            nameof(LoggedScriptHashes),
+            Array.Empty<string>()
+        );
+        set => Set(nameof(LoggedScriptHashes), value);
+    }
+
+    /// <summary>
+    /// Gets the interval to persist the logged script hashes.
+    /// </summary>
+    public TimeSpan LoggedScriptHashesPersistInterval => GetOrDefault(
+        nameof(LoggedScriptHashesPersistInterval),
+        TimeSpan.FromMinutes(5)
+    );
 }