.pr_agent_auto_best_practices

Pattern 1: When throwing exceptions, always preserve the original exception as the inner exception parameter. This maintains the full stack trace and diagnostic context, making debugging significantly easier. Apply this pattern whenever catching and re-throwing exceptions with additional context.

Example code before:

catch (JsonException ex)
{
    Logger.Error($"Failed to parse config: {file}", ex);
    throw new Exception($"Config error in {file}");
}

Example code after:

catch (JsonException ex)
{
    Logger.Error($"Failed to parse config: {file}", ex);
    throw new Exception($"Config error in {file}", ex);
}

Relevant past accepted suggestions:

---

Suggestion 1:

Preserve original exception details

Preserve the original exception's details by passing the caught JsonException as the inner exception when throwing a new Exception.

Libraries/SPTarkov.Server.Core/Servers/ConfigServer.cs [90-94]

 catch (JsonException ex)
 {
     Logger.Error($"Config file: {file} failed to deserialize", ex);
-    throw new Exception($"Server will not run until the: {file} config error mentioned above is fixed");
+    throw new Exception($"Server will not run until the: {file} config error mentioned above is fixed", ex);
 }

---

Pattern 2: Replace generic Exception throws with specific exception types (ArgumentOutOfRangeException, ArgumentNullException, InvalidOperationException, etc.) that include parameter names and contextual information. This provides clearer diagnostics and aligns with .NET framework conventions for exception handling.

Example code before:

if (value <= 0)
{
    throw new Exception($"Invalid value for {nameof(MaxMemory)}: {value}");
}

Example code after:

if (value <= 0)
{
    throw new ArgumentOutOfRangeException(nameof(MaxMemory), value, "Must be greater than zero.");
}

Relevant past accepted suggestions:

---

Pattern 3: Use TryGetValue pattern to avoid redundant dictionary lookups. When checking dictionary key existence and then accessing the value, capture the value in the TryGetValue call instead of performing a second lookup. This improves performance and reduces code duplication.

Example code before:

if (_cache.ContainsKey(key))
{
    var value = _cache[key];
    ProcessValue(value);
}

Example code after:

if (_cache.TryGetValue(key, out var value))
{
    ProcessValue(value);
}

Relevant past accepted suggestions:

---

Pattern 4: Add null checks and validation before performing operations on potentially null objects, especially when using LINQ FirstOrDefault or similar methods. Avoid using the null-forgiving operator (!) on potentially null references. Use GetRequiredService instead of GetService with null-forgiving to fail fast with clear errors.

Example code before:

var service = app.Services.GetService<MyService>()!;
await service.Initialize();

var item = collection.FirstOrDefault(x => x.Id == id);
ProcessItem(item.Value);

Example code after:

var service = app.Services.GetRequiredService<MyService>();
await service.Initialize();

var item = collection.FirstOrDefault(x => x.Id == id);
if (item?.Value is null)
{
    throw new InvalidOperationException($"Item with id {id} not found");
}
ProcessItem(item.Value);

Relevant past accepted suggestions:

---

[Auto-generated best practices - 2025-11-10]