Two compounding bugs, both hit while adding a DuckDNS provider:
1. Domain.get() normalized every lookup via tldExtract before
checking Redis. model-redis' Table.create() stores the new record
under the literal key it's given, then calls this.get() internally
to return the created instance — so for any domain tldExtract
doesn't recognize as a shared/public suffix (e.g. a DuckDNS name
like "myhost.duckdns.org", which tldExtract naively normalizes to
"duckdns.org"), that final read-back always missed and create()
threw EntryNotFound, despite the record having just been written
successfully. In other words: no *.duckdns.org domain could ever
be created. Fixed by trying the exact string first and only
falling back to the tldExtract-normalized parent if no exact
record exists — preserving the original "look up an arbitrary
hostname, find the Domain that governs it" behavior for real
lookups, while fixing create()'s own read-back of what it just
wrote.
2. create() saves the DnsProvider row first, then calls
updateDomains() as a separate step. If updateDomains() throws —
e.g. a Domain collides with a stale/orphaned record left over from
an earlier failed attempt (exactly what bug 1 was silently
producing) — the already-saved provider row was never cleaned up,
leaving a broken, domain-less provider behind despite the API
returning an error. Fixed by wrapping updateDomains() in its own
try/catch and removing the provider on failure.
That fix has its own subtlety: `instance` (from super.create())
has its `domains` relation resolved by super.create()'s own
internal get() call, which runs BEFORE updateDomains() creates any
Domain rows — so instance.domains is permanently stale (always
empty), on both the success and failure paths. Removing `instance`
directly would delete the provider but silently leave behind
whatever domains updateDomains() did manage to create. Fixed by
re-fetching (this.get(instance.id)) before both the success return
and the failure-path remove(), so relations are current in both
cases — the returned/API-response instance and the rollback's
cascade-delete.
Manually verified against a live Redis (this project's test
philosophy explicitly excludes Redis-ORM-dependent tests from the
automated suite — see test/README.md "Philosophy"):
- A pre-existing orphaned Domain (simulating bug 1's fallout) now
produces an accurate "already exists" error instead of a confusing
EntryNotFound for the wrong (normalized) domain name, and the
failed create() leaves zero orphaned providers behind.
- A genuinely new *.duckdns.org domain now creates successfully,
correctly links to its provider, and is fully cascade-deleted when
the provider is removed.
- npm test: 192/192 pass.
wmantly
73c0f85ff7