Repository navigation
report an unexpected Telnet send failure instead of dropping the client - #331
Conversation
#331 SocketHandler.Send read every non-fatal exception as a hung up connection, so a defect in what we write cost every client in turn. That is how f013 became a mass disconnect. * Only SocketException, IOException and ObjectDisposedException disconnect. Anything else is raised after the loop and reported by BackgroundSender. * SocketHandler is protected, so a subclass calling Send sees exceptions it did not before.
Five tests provoked log4net:ERROR or WARN on purpose and let it reach stderr. * AdoNetAppenderTest twice, XmlConfiguratorTest, LogLogTest and StringFormatTest once each. * Ten messages down to two. The two left are LogLogTest.EmitInternalMessages, which asserts the console output itself.
|
Implementation plan for #331, kept here for the record. Revised after the merge so that it describes the change as it landed. PlanPR #331: stop reading every exception as a hung-up clientThe defect
try
{
client.Send(message);
}
catch (Exception e) when (!e.IsFatal())
{
// The client has closed the connection, remove it from our list
client.Dispose();
RemoveClient(client);
}Any non-fatal exception is read as "this client hung up", and the client is disconnected with The agent instructions already name this: "A catch of Two commits: the fix, and a test-only pass over the provoked internal logging. Why it is safe to narrow now, and was not beforeSince #318 nothing calls catch (Exception e) when (!e.IsFatal())
{
CountLost();
Report($"[{_name}] Failed to send a logging event.", e);
}So an exception that escapes Commit 1: the change
The comment in the
Commit 1: the test
Revert to fail, in place, not with The thing to note, not silently accept
Commit 2: the internal logging that reaches the consoleUnrelated to the catch, but test hygiene on the same rule, and small. Measured on
Ten messages down to two. The two left are Also in this commit: the agent instructions get the Changelog
VerificationHouse rulesBranch |
Every throw from a Telnet write was read as a dead client, so a bug in what we write dropped every
client, one per event. That was f013.
Only
SocketException,IOExceptionandObjectDisposedExceptiondisconnect now. The rest isrethrown after the loop and reported by
BackgroundSender.Commit 2, test-only:
AdoNetAppenderTesttwice,XmlConfiguratorTest,LogLogTest,StringFormatTestonce eachlog4net:ERRORoff stderr, two left, asserted byLogLogTestCLAUDE.mdgets theReflectionExtensionsconvention from one home for the reflection in the tests #326