ISSUE-035: accept perpetuum.ini files carrying obsolete connection string keywords - #26
Conversation
|
Flagging the one thing in here that deserves a deliberate yes or no rather than a skim: This changes behaviour. A If you would rather the server kept refusing that file, the alternative is a couple of lines away and What does not change either way: a connection string carrying none of these keywords is handed |
|
Direction confirmed with the maintainers on Discord on 2026-08-15. Recording it here so the decision is on the record rather than only in chat. The obsolete keyword should be reported, not removed. The reasoning given: the installer already deploys a corrected So this PR changes shape. Instead of stripping the keyword, One assumption in that, stated plainly so it can be overruled in one line: the wording was "check and log the error". Logging alone would leave the server to fail a moment later inside Please hold this PR — the rework is not pushed yet. The unit tests for this code exist on a local branch and their expectations need to be inverted to match the new behaviour. I will push both together and update this thread. |
…eywords (ISSUE-035)
The perpetuum.ini written by the Perpetuum Dedicated Server installer does not
start this server. It was written for the original server, which used
System.Data.SqlClient; this one uses Microsoft.Data.SqlClient, which differs in
two ways that both abort startup before any zone loads.
Measured against Microsoft.Data.SqlClient 6.0.1 with neutral resources, so the
messages below are the ones a maintainer sees rather than a translation:
Connection Reset=True
System.NotSupportedException: The keyword 'Connection Reset' is not
supported on this platform.
thrown while SqlConnection is being constructed, not on Open().
no Encrypt / TrustServerCertificate
SqlException on Open(): A connection was successfully established with the
server, but then an error occurred during the login process. (provider: SSL
Provider, error: 0 - ...)
Neither message names perpetuum.ini, which is what makes a fresh setup hard to
diagnose.
The first is now handled. LegacyConnectionString.RemoveObsoleteKeywords drops
keywords that Microsoft.Data.SqlClient refuses AND that the framework had
already stopped honouring, so removing them cannot change how the server
connects. PerpetuumBootstrapper.Init calls it directly after resolving
GlobalConfiguration — before anything constructs a SqlConnection — and logs a
warning naming perpetuum.ini and the keyword.
Keywords that still carry meaning are deliberately left alone. Network Library
selects a protocol and Context Connection selects a SQLCLR connection; both
need an operator decision, and the driver's own error already names the
replacement.
Two implementation notes, both measured rather than assumed:
- Parsing goes through DbConnectionStringBuilder, the provider-agnostic
parser. SqlConnectionStringBuilder throws on the same keyword, so it cannot
be used to find it. Splitting on ';' by hand was rejected because it
corrupts quoted values containing a separator, verified against
Password="a;b=c".
- The input is returned untouched when nothing is removed. Rebuilding through
DbConnectionStringBuilder lower-cases every key and drops the trailing
separator, and a connection string that reaches a log should be the one the
operator wrote.
The second failure is documentation only. Defaulting TrustServerCertificate in
code would weaken authentication for every operator in order to fix a local
development case. README.md gains a setup section instead: the requirement, a
working connection string, and the caveat that TrustServerCertificate=True
belongs on a local instance only.
Verified against a real server run in three states:
Connection Reset present, fix applied warning naming perpetuum.ini and the
keyword, then Database: perpetuumsa
Connection Reset present, fix reverted the NotSupportedException above, the
process exits, no mention of the file
keyword absent, fix applied no warning, log identical to before
The third state is the one that shows valid configurations are unaffected.
Maintainer direction, given on Discord on 2026-08-15: the installer already deploys a corrected perpetuum.ini, so this only bites when a fresh PP2 is pointed at an old PP1 data folder, and a clear diagnostic serves that case better than silently accepting the file. This is also what ISSUE-035's own Proposed Fix asked for: "an error that names perpetuum.ini and the offending key". LegacyConnectionString is replaced by ConnectionStringSupport, which reports and never rewrites. The connection string reaches the driver exactly as the operator wrote it. PerpetuumBootstrapper.Init now logs an error naming perpetuum.ini, the directory it sits in, and every offending setting, then throws so the server does not start on a connection that cannot open. There is no precedent for refusing to start in this file -- it holds no other throw -- so the choice of InvalidOperationException is stated in the pull request for the maintainers to overrule. The check keeps no keyword list. It parses with DbConnectionStringBuilder, then offers each setting to SqlConnectionStringBuilder and reports whatever that refuses. Two consequences: Network Library and Context Connection are now named, where removal had to leave them alone because dropping them would change how the server connects; and a setting nobody anticipated is reported the same way. Every offending setting is reported in one message. The installer's file carries more than one, so a first-failure-only report would cost the operator a restart per setting. The catch around the per-setting probe is deliberately broad. Measured against Microsoft.Data.SqlClient 6.0.1, the driver uses three different exception types for this -- NotSupportedException for Connection Reset and Network Library, ArgumentException for Asynchronous Processing, InvalidOperationException for Context Connection -- and a narrow filter that missed one would report the setting as supported and hand the operator the obscure failure this check exists to replace. Verified: - 15 unit tests in ConnectionStringSupportTests, 8 of them observed failing against a stub before the detection was written - full unit tier green, 73/73 - solution builds with 0 errors, and no warning comes from a project this touches - a game root carrying Connection Reset and Network Library produces one ERR line naming both and exits 1, before anything else in Init needs the game root - tools/smoke-test.ps1 green against the real database with a good file: [Online] after 80s, 6435 members spawned, graceful shutdown, exit 0 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fc93d13 to
b14cf40
Compare
|
The rework is pushed — this PR now reports the rejected settings and refuses to start, instead of removing them. The branch was also rebased onto the current Read the second commit on its own to see the change of direction; the first is the original work, unchanged apart from the rebase. What it does now
Against a game root carrying two bad settings: Process exits 1. The check runs before anything else in Two decisions worth a lookNo keyword list. The check parses with
Verification
The three settings-reporting details — all offenders in one message rather than one per restart, the deliberately broad |
Fixes
ISSUE-035: theperpetuum.iniwritten by the Perpetuum Dedicated Server installer does not start this server, and neither error message names the file.This PR changed direction on 2026-08-15 after the maintainers said on Discord that they would rather see the bad setting reported than silently removed. The original approach removed it. The two comments below record that decision and the rework; this body describes what the branch does now.
The problem
The installer's file was written for the original server, which used
System.Data.SqlClient. This one usesMicrosoft.Data.SqlClient, which refuses settings the old driver accepted.SqlConnectionthrows while it is being constructed, naming the setting but not the file it came from — and the file is the part an operator needs.What this does
ConnectionStringSupport.FindUnsupportedKeywordsreports every setting the driver will refuse. Nothing is ever rewritten: the connection string reaches the driver exactly as it was written.PerpetuumBootstrapper.Initcalls it right after resolvingGlobalConfiguration, and when anything comes back it logs an error namingperpetuum.ini, the directory it sits in, and every offending setting, then throws so the server does not start on a connection that cannot open.No keyword list. The check parses with
DbConnectionStringBuilder, then offers each setting toSqlConnectionStringBuilderand reports whatever it refuses. The old list was short only because removing a setting had to be provably safe; reporting always is. SoNetwork LibraryandContext Connectionare named too, and a setting nobody anticipated is handled the same way.All offenders in one message, because the installer's file carries more than one and a first-failure-only report costs a restart per setting.
The
Encrypt/TrustServerCertificatehalf ofISSUE-035stays documentation only: defaultingTrustServerCertificatewould weaken authentication for every operator to fix a local development case.README.mdgains a setup section — the repository had none — stating the requirement, a working connection string, and thatTrustServerCertificate=Truebelongs on a local instance only.The one decision I would most like overruled if it is wrong
InvalidOperationException.CLAUDE.mdpoints atPerpetuumException/ErrorCodes, but those model game protocol errors and this is a startup configuration problem, so adding anErrorCodefor it seemed heavier than the problem. There is no precedent to copy either:PerpetuumBootstrapper.cscontains no otherthrow, so refusing to start is new behaviour for that file however it is spelled.Verification
ConnectionStringSupportTests; 8 observed failing against a stub before the detection was writtenERRline naming both settings, process exits 1, before anything else inInitneeds the game roottools/smoke-test.ps1green against a real database —[Online]after 80 s, 6435 members spawned, graceful shutdown, exit 0The_driver_really_does_reject_the_installer_stringfails the day the driver stops rejecting the keyword, which is the day this check becomes dead weightBranch rebased onto the current
develop, so the first commit's hash changed. Reading the second commit on its own shows the change of direction.