Einstellungen: erst schreiben, dann uebernehmen
CI / Build & Tests (Linux) (push) Canceled after 0s
CI / Plattformneutralität & Hygiene (push) Canceled after 0s

Behebt #534.

Die vier schreibenden Endpunkte verschluckten die Ausnahme des Repositories und
meldeten anschliessend unbedingt Erfolg. Das Verschlucken war unschoen; die
Reihenfolge war gefaehrlich: copyState wurde auch dann geaendert, wenn die
Datenbank abgelehnt hatte. Das Modul handelte danach mit Risikolimits, die
nirgends standen, und nach dem naechsten Neustart konnte niemand mehr
rekonstruieren, womit es gehandelt hatte.

Neu geht der Schreibvorgang zuerst, und der Hauptspeicher wird nur nachgezogen,
wenn er gelang. Sonst 500 mit Klartext und unveraenderter Zustand. Fehlt die
Ablage ganz, ist die Aenderung ebenso rein fluechtig — auch das wird abgelehnt,
mit 503.

Betroffen: CopyTrading-Kontoeinstellungen, ResolutionFarming-Kontoeinstellungen,
Master-Trader anlegen/aendern, Master-Trader loeschen.

Dass alle vier denselben Helfer (PersistenceGuard) benutzen, ist der eigentliche
Punkt: Aus je eigenem try/catch je Endpunkt ist die Ungleichheit ueberhaupt erst
entstanden.

Bewusst anders behandelt bleibt das Archivieren eines Supervisor-Berichts: Die
Antwort des Agenten steht dort bereits und hat Geld gekostet: sie wegen eines
fehlgeschlagenen Archiveintrags zu verwerfen, waere schlimmer als der fehlende
Eintrag, und an diesem Schreibvorgang haengt kein Verhalten des Systems.
Stillschweigend verschwindet er aber nicht mehr, sondern wird protokolliert.

In der Oberflaeche zeigten die Schreibwege bisher nur "HTTP 500" an und
verschenkten damit genau die Begruendung, die der Server jetzt mitliefert.
errorText() liest sie aus der Antwort.

Entscheidung von Richard: ablehnen statt fluechtig uebernehmen. Damit sind
waehrend eines Datenbankausfalls keine Aenderungen an den Einstellungen mehr
moeglich; das Handeln selbst laeuft weiter.

Geprueft auf der Linux-Test-VM mit angehaltener MariaDB: alle drei Schreibwege
antworten mit 500 und nennen den Grund, das Limit bleibt bei 11,25 statt auf
99,99 zu springen, der Phantom-Trader entsteht weder im Hauptspeicher noch in
der Datenbank, jeder Fehlschlag steht im Protokoll. Nach dem Wiederanlauf
speichert es wieder. 556 von 556 Tests bestanden (5 neue).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Richard
2026-09-10 15:51:16 +02:00
co-authored by Claude Opus 5
parent 72ad38518b
commit 4c556be57d
6 changed files with 188 additions and 28 deletions
@@ -8,6 +8,7 @@ using Microsoft.Extensions.DependencyInjection;
using PolyTrader.Modules.CopyTrading.Persistence; using PolyTrader.Modules.CopyTrading.Persistence;
using PolyTraderSharp; using PolyTraderSharp;
using PolyTraderSharp.Models; using PolyTraderSharp.Models;
using PolyTraderSharp.Services;
namespace PolyTrader.Web.Endpoints namespace PolyTrader.Web.Endpoints
{ {
@@ -60,7 +61,7 @@ namespace PolyTrader.Web.Endpoints
}); });
// 2. Master-Trader anlegen / aktualisieren // 2. Master-Trader anlegen / aktualisieren
group.MapPost("/traders", (IServiceProvider sp, CopyTradingState copyState, TrackedTraderInput input) => group.MapPost("/traders", (IServiceProvider sp, CopyTradingState copyState, TerminalLogger logger, TrackedTraderInput input) =>
{ {
var repo = sp.GetService<ITrackedTraderRepository>(); var repo = sp.GetService<ITrackedTraderRepository>();
@@ -109,12 +110,11 @@ namespace PolyTrader.Web.Endpoints
trader.CopyTradeCount30d = existingTrader.CopyTradeCount30d; trader.CopyTradeCount30d = existingTrader.CopyTradeCount30d;
} }
if (repo != null) var failed = PersistenceGuard.Write(repo != null, () => repo!.Upsert(trader), logger,
{ $"Master-Trader #{id} ({trader.DisplayName})");
try { repo.Upsert(trader); } if (failed != null) return failed;
catch { /* Fallback in-memory */ }
}
// Erst nach erfolgreichem Schreiben in den Hauptspeicher (#534).
copyState.Traders[id] = trader; copyState.Traders[id] = trader;
return Results.Ok(new return Results.Ok(new
@@ -138,14 +138,13 @@ namespace PolyTrader.Web.Endpoints
}); });
// 3. Master-Trader löschen // 3. Master-Trader löschen
group.MapDelete("/traders/{id:int}", (IServiceProvider sp, CopyTradingState copyState, int id) => group.MapDelete("/traders/{id:int}", (IServiceProvider sp, CopyTradingState copyState, TerminalLogger logger, int id) =>
{ {
var repo = sp.GetService<ITrackedTraderRepository>(); var repo = sp.GetService<ITrackedTraderRepository>();
if (repo != null)
{ var failed = PersistenceGuard.Write(repo != null, () => repo!.Delete(id), logger,
try { repo.Delete(id); } $"Loeschen von Master-Trader #{id}");
catch { } if (failed != null) return failed;
}
copyState.Traders.TryRemove(id, out _); copyState.Traders.TryRemove(id, out _);
return Results.Ok(new { success = true, id }); return Results.Ok(new { success = true, id });
@@ -334,17 +333,17 @@ namespace PolyTrader.Web.Endpoints
group.MapPost("/settings/{accountId:int}", ( group.MapPost("/settings/{accountId:int}", (
IServiceProvider sp, IServiceProvider sp,
CopyTradingState copyState, CopyTradingState copyState,
TerminalLogger logger,
int accountId, int accountId,
CopyTradingAccountSettings input) => CopyTradingAccountSettings input) =>
{ {
input.AccountId = accountId; input.AccountId = accountId;
var repo = sp.GetService<ICopyTradingAccountSettingsRepository>(); var repo = sp.GetService<ICopyTradingAccountSettingsRepository>();
if (repo != null)
{ var failed = PersistenceGuard.Write(repo != null, () => repo!.Upsert(input), logger,
try { repo.Upsert(input); } $"CopyTrading-Einstellungen fuer Konto {accountId}");
catch { } if (failed != null) return failed;
}
copyState.AccountSettings[accountId] = input; copyState.AccountSettings[accountId] = input;
@@ -0,0 +1,56 @@
using System;
using Microsoft.AspNetCore.Http;
using PolyTraderSharp.Services;
namespace PolyTrader.Web.Endpoints
{
/// <summary>
/// Gemeinsamer Schreibweg für die Endpunkte, die etwas dauerhaft ablegen (#534).
///
/// <para><b>Erst schreiben, dann übernehmen.</b> Vorher aktualisierte jeder Endpunkt den
/// Hauptspeicher auch dann, wenn die Datenbank abgelehnt hatte, und meldete trotzdem Erfolg.
/// Bei Risikolimits ist das der teuerste Zustand: Das Modul handelt mit Werten, die nirgends
/// stehen, und nach dem nächsten Neustart kann niemand mehr rekonstruieren, womit es gehandelt
/// hat. Ein sichtbar abgelehnter Speichervorgang ist ärgerlich und folgenlos — ein halb
/// angewandter ist unsichtbar und folgenreich.</para>
///
/// <para>Dass alle vier Schreibwege denselben Helfer benutzen, ist der eigentliche Punkt: Aus je
/// eigenem <c>try</c>/<c>catch</c> je Endpunkt ist die Ungleichheit überhaupt erst entstanden.</para>
/// </summary>
internal static class PersistenceGuard
{
/// <summary>
/// Führt <paramref name="write"/> aus. Gibt <c>null</c> zurück, wenn es gelang — dann darf der
/// Aufrufer den Hauptspeicher nachziehen. Sonst die fertige Fehlerantwort, die unverändert
/// zurückzugeben ist; der Hauptspeicher bleibt dabei unberührt.
/// </summary>
/// <param name="repositoryPresent">
/// Ob überhaupt eine Ablage vorhanden ist. Fehlt sie, ist das Modul nicht vollständig geladen —
/// eine Änderung wäre rein flüchtig und wird deshalb ebenso abgelehnt.
/// </param>
public static IResult? Write(bool repositoryPresent, Action write, TerminalLogger logger, string what)
{
if (!repositoryPresent)
{
logger.Error($"{what}: keine Ablage verfügbar – nicht gespeichert.");
return Results.Problem(
detail: $"{what} konnte nicht gespeichert werden: Die Ablage ist nicht verfügbar. " +
"Es wurde nichts geändert.",
statusCode: StatusCodes.Status503ServiceUnavailable);
}
try
{
write();
return null;
}
catch (Exception ex)
{
logger.Error($"{what}: nicht gespeichert – {ex.Message}");
return Results.Problem(
detail: $"{what} konnte nicht gespeichert werden: {ex.Message} Es wurde nichts geändert.",
statusCode: StatusCodes.Status500InternalServerError);
}
}
}
}
@@ -8,6 +8,7 @@ using Microsoft.Extensions.DependencyInjection;
using PolyTrader.Modules.ResolutionFarming.Models; using PolyTrader.Modules.ResolutionFarming.Models;
using PolyTrader.Modules.ResolutionFarming.Persistence; using PolyTrader.Modules.ResolutionFarming.Persistence;
using PolyTraderSharp; using PolyTraderSharp;
using PolyTraderSharp.Services;
namespace PolyTrader.Web.Endpoints namespace PolyTrader.Web.Endpoints
{ {
@@ -217,16 +218,15 @@ namespace PolyTrader.Web.Endpoints
}); });
// 5. Einstellungen speichern // 5. Einstellungen speichern
group.MapPost("/settings/{accountId:int}", (IServiceProvider sp, int accountId, RfSettings input) => group.MapPost("/settings/{accountId:int}", (IServiceProvider sp, TerminalLogger logger, int accountId, RfSettings input) =>
{ {
input.AccountId = accountId; input.AccountId = accountId;
var repo = sp.GetService<IRfSettingsRepository>(); var repo = sp.GetService<IRfSettingsRepository>();
if (repo != null)
{ var failed = PersistenceGuard.Write(repo != null, () => repo!.Upsert(input), logger,
try { repo.Upsert(input); } $"ResolutionFarming-Einstellungen fuer Konto {accountId}");
catch { } if (failed != null) return failed;
}
return Results.Ok(new return Results.Ok(new
{ {
@@ -78,7 +78,16 @@ namespace PolyTrader.Web.Endpoints
}; };
reportRepo.Insert(report); reportRepo.Insert(report);
} }
catch { /* Ignoriert bei DB-Offline */ } catch (Exception ex)
{
// Bewusst nicht abbrechen (#534): Die Antwort des Agenten steht bereits und
// hat Geld gekostet — sie wegen eines fehlgeschlagenen Archiveintrags zu
// verwerfen, waere schlimmer als der fehlende Eintrag. Anders als bei
// Risikolimits haengt an diesem Schreibvorgang kein Verhalten des Systems.
// Stillschweigend darf er aber auch nicht verschwinden.
sp.GetService<PolyTraderSharp.Services.TerminalLogger>()
?.Error($"Supervisor-Bericht nicht archiviert: {ex.Message}");
}
} }
return Results.Ok(new return Results.Ok(new
+17 -4
View File
@@ -524,6 +524,19 @@ function escapeHtml(str) {
// Zeichenkette als JS-Literal in ein onclick-Attribut. escapeHtml allein genuegt // Zeichenkette als JS-Literal in ein onclick-Attribut. escapeHtml allein genuegt
// dort nicht: Es laesst ' unberuehrt, und ein Name wie O'Brien beendet das // dort nicht: Es laesst ' unberuehrt, und ein Name wie O'Brien beendet das
// Literal vorzeitig - der Knopf ist dann tot. // Literal vorzeitig - der Knopf ist dann tot.
/**
* Fehlertext einer abgelehnten Antwort. Die Schreibwege liefern seit #534 eine Begruendung
* mit ("... Es wurde nichts geaendert."); ein blosses "HTTP 500" verschenkt genau die.
*/
async function errorText(res) {
try {
const body = await res.json();
return body.detail || body.error || body.title || `HTTP ${res.status}`;
} catch (e) {
return `HTTP ${res.status}`;
}
}
function jsArg(str) { function jsArg(str) {
return escapeHtml(JSON.stringify(String(str === undefined || str === null ? '' : str))); return escapeHtml(JSON.stringify(String(str === undefined || str === null ? '' : str)));
} }
@@ -1138,7 +1151,7 @@ async function saveTraderModal(e) {
headers: { 'Content-Type': 'application/json' }, headers: { 'Content-Type': 'application/json' },
body: JSON.stringify(payload) body: JSON.stringify(payload)
}); });
if (!res.ok) throw new Error('HTTP ' + res.status); if (!res.ok) throw new Error(await errorText(res));
closeTraderModal(); closeTraderModal();
loadCtTraders(); loadCtTraders();
} catch (err) { } catch (err) {
@@ -1150,7 +1163,7 @@ async function deleteTrader(id, name) {
if (!confirm(`Master-Trader #${id} (${name}) wirklich löschen?`)) return; if (!confirm(`Master-Trader #${id} (${name}) wirklich löschen?`)) return;
try { try {
const res = await fetch(`/api/modules/copytrading/traders/${id}`, { method: 'DELETE' }); const res = await fetch(`/api/modules/copytrading/traders/${id}`, { method: 'DELETE' });
if (!res.ok) throw new Error('HTTP ' + res.status); if (!res.ok) throw new Error(await errorText(res));
loadCtTraders(); loadCtTraders();
} catch (err) { } catch (err) {
alert('Fehler beim Löschen: ' + err.message); alert('Fehler beim Löschen: ' + err.message);
@@ -1381,7 +1394,7 @@ async function saveCtSettings(e) {
headers: { 'Content-Type': 'application/json' }, headers: { 'Content-Type': 'application/json' },
body: JSON.stringify(payload) body: JSON.stringify(payload)
}); });
if (!res.ok) throw new Error('HTTP ' + res.status); if (!res.ok) throw new Error(await errorText(res));
const statusEl = document.getElementById('ctSettingsStatus'); const statusEl = document.getElementById('ctSettingsStatus');
if (statusEl) { if (statusEl) {
@@ -1666,7 +1679,7 @@ async function saveRfSettings(e) {
headers: { 'Content-Type': 'application/json' }, headers: { 'Content-Type': 'application/json' },
body: JSON.stringify(payload) body: JSON.stringify(payload)
}); });
if (!res.ok) throw new Error('HTTP ' + res.status); if (!res.ok) throw new Error(await errorText(res));
const statusEl = document.getElementById('rfSettingsStatus'); const statusEl = document.getElementById('rfSettingsStatus');
if (statusEl) { if (statusEl) {
@@ -0,0 +1,83 @@
using System;
using System.Collections.Generic;
using Microsoft.AspNetCore.Http;
using PolyTrader.Web.Endpoints;
using PolyTraderSharp.Services;
using Xunit;
namespace PolyTrader.Tests.WebUi
{
/// <summary>
/// Der gemeinsame Schreibweg der Einstellungs-Endpunkte (#534).
///
/// Der Kern ist nicht „meldet einen Fehler", sondern <b>„lässt den Hauptspeicher in Ruhe"</b>:
/// Vorher wurde die Änderung auch nach einem gescheiterten Schreibvorgang übernommen und als
/// Erfolg gemeldet. Bei Risikolimits heißt das, dass das Modul mit Werten handelt, die nirgends
/// stehen und nach einem Neustart nicht mehr rekonstruierbar sind.
/// </summary>
public class PersistenceGuardTests
{
private static TerminalLogger Logger() => new();
[Fact]
public void Gelungenes_Schreiben_gibt_keine_Fehlerantwort()
{
bool geschrieben = false;
var result = PersistenceGuard.Write(true, () => geschrieben = true, Logger(), "Testwert");
Assert.Null(result); // null = der Aufrufer darf nachziehen
Assert.True(geschrieben);
}
[Fact]
public void Ausnahme_beim_Schreiben_liefert_eine_Fehlerantwort()
{
var result = PersistenceGuard.Write(
true,
() => throw new InvalidOperationException("Verbindung weg"),
Logger(),
"CopyTrading-Einstellungen fuer Konto 1");
Assert.NotNull(result);
Assert.IsAssignableFrom<IStatusCodeHttpResult>(result);
Assert.Equal(StatusCodes.Status500InternalServerError, ((IStatusCodeHttpResult)result!).StatusCode);
}
[Fact]
public void Fehlende_Ablage_wird_ebenso_abgelehnt()
{
bool geschrieben = false;
// Ohne Repository waere die Aenderung rein fluechtig — also dasselbe Problem.
var result = PersistenceGuard.Write(false, () => geschrieben = true, Logger(), "Testwert");
Assert.NotNull(result);
Assert.Equal(StatusCodes.Status503ServiceUnavailable, ((IStatusCodeHttpResult)result!).StatusCode);
Assert.False(geschrieben);
}
/// <summary>
/// Das eigentliche Versprechen, nachgestellt am Ablauf des Endpunkts: Erst schreiben, den
/// Hauptspeicher nur bei Erfolg nachziehen.
/// </summary>
[Theory]
[InlineData(true)]
[InlineData(false)]
public void Hauptspeicher_wird_nur_bei_Erfolg_nachgezogen(bool schreibenGelingt)
{
var hauptspeicher = new Dictionary<int, decimal> { [1] = 5.00m };
var result = PersistenceGuard.Write(
true,
() => { if (!schreibenGelingt) throw new InvalidOperationException("DB weg"); },
Logger(),
"Risikolimit Konto 1");
if (result == null)
hauptspeicher[1] = 7.50m;
Assert.Equal(schreibenGelingt ? 7.50m : 5.00m, hauptspeicher[1]);
}
}
}