From 317196fd33768e53b6ef39cca170d2aecc3b9002 Mon Sep 17 00:00:00 2001 From: Gabriel Rosales Date: Thu, 20 Aug 2026 16:59:10 -0600 Subject: [PATCH 1/2] fix(config)!: address a database section's errors to that section Root, databases., and multitenant.tenants..database share one normalization module and therefore one set of error constructors, all spelling their fields against the root. The startup door attached the section path by WRAPPING, so the message read "databases.reporting: config_missing: database.database required" while the *ConfigError a consumer reaches through errors.As still said Field="database.database". A consumer switching on Field could not tell which section failed, and the spelling disagreed with the path-qualified keys ADR-051's delivered-empty check already emits for the same sections. normalizeDatabaseSection now rewrites the error it returns: a key under the root section swaps its "database" head for the section path (databases.reporting.host), and a tenant keeps its own trailing ".database" (multitenant.tenants.acme.database.host) -- byte-identical to the delivered-empty spelling, cross-checked by a test over every identity key rather than by two hand-typed tables agreeing. A field that is not key-shaped is prefixed instead, so the Oracle connection- identifier check keeps its name. The rewrite works on a copy, because the constructors are shared with the connect door, which resolves a config with no section path to speak of; that door keeps the root spelling and is pinned by its own test. The wrapping message is gone, so the path is printed once. Two adjacent defects were found while writing this and are filed, not fixed: the runtime door still reports the root spelling for a dynamically-resolved tenant, alongside three other Field spellings in the tenant tree (#1113), and Action still names the root env var for a named section, which the removed wrapper used to frame as a template (#1114). ADR-076 records both deferrals and their cost. Decision recorded in ADR-076; migration atom C60.16. Closes #1025 --- CLAUDE.md | 1 + config/database_section.go | 48 +++++- config/database_section_test.go | 154 +++++++++++++++++- ...76_section_qualified_config_error_field.md | 100 ++++++++++++ wiki/architecture_decisions.md | 21 ++- wiki/migrations.md | 43 ++++- 6 files changed, 358 insertions(+), 9 deletions(-) create mode 100644 wiki/adr_076_section_qualified_config_error_field.md diff --git a/CLAUDE.md b/CLAUDE.md index b05f6fae..8f446e45 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -424,6 +424,7 @@ GoBricks breaks its own API surface when justified. Greenfield work uses the new - **Dead app lifecycle surface removed (ADR-067):** `MessagingInitializer` and `ConnectionPreWarmer` (constructors and methods included), `Options.Database` and `Options.MessagingClient` are gone; the eight debug response types are unexported with their JSON unchanged. - **One delivery pipeline (ADR-068):** `messaging.StartConsumeSpan` is removed — a service driving its own consume loop starts its own span — and the AMQP `messaging.client.consumed.messages` counter is recorded at completion with `error.type` instead of at receive without it. - **Scheduler timeouts normalize (ADR-075):** `scheduler.timeout.shutdown`/`slowjob` default in `config.Validate` (30s/25s — hand-built configs move 30s → 25s), negatives fail validation, and `scheduler.Module.Init` requires a NORMALIZED `deps.Config` — non-nil, both timeouts positive — so a config assembled outside app construction must go through `config.Validate` first. +- **Section-qualified config errors (ADR-076):** a non-root database section's `ConfigError.Field` names that section (`databases.reporting.host`, `multitenant.tenants.acme.database.host`); match by suffix, not equality. Root and the connect door keep the root spelling. ## File Organization diff --git a/config/database_section.go b/config/database_section.go index 9baafa1b..5977fe67 100644 --- a/config/database_section.go +++ b/config/database_section.go @@ -1,9 +1,11 @@ package config import ( + "errors" "fmt" "maps" "slices" + "strings" ) // dbPlacement is where a database section sits in the configuration tree. It @@ -145,10 +147,7 @@ func normalizeDatabaseSection(db *DatabaseConfig, section dbSection) error { } if err := normalizeDatabaseValues(db, dbStrictnessStartup); err != nil { - if section.placement == dbPlacementRoot { - return err - } - return fmt.Errorf("%s: %w", section.path, err) + return section.qualify(err) } if section.placement != dbPlacementRoot && db.Manager.isSet() { @@ -162,6 +161,47 @@ func normalizeDatabaseSection(db *DatabaseConfig, section dbSection) error { return nil } +// qualify re-addresses an error raised against the root spelling to this section, so a +// consumer matching on ConfigError.Field learns WHICH section failed. The root section +// returns the error untouched; every other placement gets a rewritten copy — the original +// is left alone because normalization shares its error constructors with the connect door, +// which has no section path to speak of. +// +// The path is carried by Field alone, never also by a wrapping message: printing it in both +// places is how the same section path ends up in one error twice. +// +// Action is deliberately left as the root spelling names it. Rewriting operator hints is a +// different problem from addressing an error, and it is tracked separately — see ADR-076. +func (s dbSection) qualify(err error) error { + if s.placement == dbPlacementRoot { + return err + } + var cfgErr *ConfigError + if !errors.As(err, &cfgErr) { + return fmt.Errorf("%s: %w", s.path, err) + } + qualified := *cfgErr + qualified.Field = s.qualifyField(cfgErr.Field) + qualified.Details = slices.Clone(cfgErr.Details) + return &qualified +} + +// qualifyField rewrites one root-spelled field to this section. A key under the root +// section swaps its "database" head for the section path, so "database.host" reads +// "databases.reporting.host" and the tenant spelling keeps its own trailing ".database". +// A field that is not key-shaped — the Oracle connection-identifier check names one — is +// prefixed instead, which keeps the offending name rather than dropping it. +func (s dbSection) qualifyField(field string) string { + switch { + case field == "" || field == fieldDatabase: + return s.path + case strings.HasPrefix(field, fieldDatabase+"."): + return s.path + strings.TrimPrefix(field, fieldDatabase) + default: + return s.path + "." + field + } +} + // forEachDatabaseSection visits every database section the deployment // consumes: the root, each databases.* entry, and — only when multitenancy is // enabled, since a leftover tenants block is inert otherwise — each static diff --git a/config/database_section_test.go b/config/database_section_test.go index bcb1836c..563fcc92 100644 --- a/config/database_section_test.go +++ b/config/database_section_test.go @@ -2,6 +2,7 @@ package config import ( "errors" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -112,8 +113,6 @@ func TestNormalizeDatabaseSectionPlacementRules(t *testing.T) { wantCategory: errCategoryInvalid, wantField: "multitenant.tenants.t.database.manager", }, - {name: "named_normalization_error_wrapped_with_path", section: namedDatabaseSection("r"), cfg: DatabaseConfig{Type: "mysql", Host: "h"}, wantErr: "databases.r: "}, - {name: "tenant_normalization_error_wrapped_with_path", section: tenantDatabaseSection("t"), cfg: DatabaseConfig{Type: "mysql", Host: "h"}, wantErr: "multitenant.tenants.t.database: "}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -134,6 +133,157 @@ func TestNormalizeDatabaseSectionPlacementRules(t *testing.T) { } } +// TestNormalizeDatabaseSectionQualifiesFieldWithSectionPath pins the addressing rule +// across every placement: a consumer matching on ConfigError.Field must be told WHICH +// section failed, not the root spelling of a key that belongs to another section. +func TestNormalizeDatabaseSectionQualifiesFieldWithSectionPath(t *testing.T) { + missingIdentity := DatabaseConfig{Type: PostgreSQL, Host: "h", Username: "u"} + typeConflict := DatabaseConfig{Type: Oracle, ConnectionString: "postgres://x/y"} + tlsMaterial := DatabaseConfig{ + Type: PostgreSQL, Host: "h", Port: 5432, Database: "d", Username: "u", + TLS: TLSConfig{CertFile: "c.pem"}, + } + // The one field the database path names that is not key-shaped. + oracleOutlier := DatabaseConfig{Type: Oracle, Host: "h", Port: 1521, Username: "u"} + + tests := []struct { + name string + section dbSection + cfg DatabaseConfig + wantField string + }{ + {name: "root_missing_identity", section: rootDatabaseSection(), cfg: missingIdentity, wantField: "database.port"}, + {name: "root_type_conflict", section: rootDatabaseSection(), cfg: typeConflict, wantField: "database.type"}, + {name: "root_tls_material", section: rootDatabaseSection(), cfg: tlsMaterial, wantField: "database.tls"}, + {name: "root_oracle_outlier", section: rootDatabaseSection(), cfg: oracleOutlier, wantField: "oracle connection identifier"}, + + {name: "named_missing_identity", section: namedDatabaseSection("reporting"), cfg: missingIdentity, wantField: "databases.reporting.port"}, + {name: "named_type_conflict", section: namedDatabaseSection("reporting"), cfg: typeConflict, wantField: "databases.reporting.type"}, + {name: "named_tls_material", section: namedDatabaseSection("reporting"), cfg: tlsMaterial, wantField: "databases.reporting.tls"}, + {name: "named_oracle_outlier", section: namedDatabaseSection("reporting"), cfg: oracleOutlier, wantField: "databases.reporting.oracle connection identifier"}, + + {name: "tenant_missing_identity", section: tenantDatabaseSection("acme"), cfg: missingIdentity, wantField: "multitenant.tenants.acme.database.port"}, + {name: "tenant_type_conflict", section: tenantDatabaseSection("acme"), cfg: typeConflict, wantField: "multitenant.tenants.acme.database.type"}, + {name: "tenant_tls_material", section: tenantDatabaseSection("acme"), cfg: tlsMaterial, wantField: "multitenant.tenants.acme.database.tls"}, + {name: "tenant_oracle_outlier", section: tenantDatabaseSection("acme"), cfg: oracleOutlier, wantField: "multitenant.tenants.acme.database.oracle connection identifier"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := tt.cfg + + err := normalizeDatabaseSection(&cfg, tt.section) + + var cfgErr *ConfigError + require.ErrorAs(t, err, &cfgErr) + assert.Equal(t, tt.wantField, cfgErr.Field) + if tt.section.placement != dbPlacementRoot { + assert.False(t, strings.HasPrefix(err.Error(), tt.section.path+": "), + "the path is carried by Field; a wrapper prefix would print it twice") + assert.Equal(t, 1, strings.Count(err.Error(), tt.section.path), + "and it appears exactly once in the rendered error") + } + }) + } +} + +// TestQualifiedFieldMatchesDeliveredEmptySpelling cross-checks the two producers instead of +// trusting that two hand-typed tables agree: ADR-076's whole point is that a key has ONE +// spelling per section, and the delivered-empty check (ADR-051) is the other producer of it. +func TestQualifiedFieldMatchesDeliveredEmptySpelling(t *testing.T) { + sections := []dbSection{ + rootDatabaseSection(), + namedDatabaseSection("reporting"), + tenantDatabaseSection("acme"), + } + + for _, section := range sections { + t.Run(section.path, func(t *testing.T) { + for _, key := range databaseIdentityKeys { + // What validateNoDeliveredEmptyDatabase reports for this key and section. + deliveredEmpty := section.path + "." + key + + // What a normalization error against the same key is re-addressed to. + qualified := section.qualifyField(fieldDatabase + "." + key) + + assert.Equal(t, deliveredEmpty, qualified, "key %q must have one spelling", key) + } + }) + } +} + +// TestNormalizeDatabaseValuesConnectKeepsRootSpelling pins the other half of the seam: +// the connect door shares these error constructors but has no section to speak of, so it +// must keep the root spelling rather than inherit a path from the startup door. +// TestDbSectionQualifyContract exercises the rewriter directly. Its two defensive +// branches cannot be reached through normalizeDatabaseSection today — every producer on +// that path returns a *ConfigError with a "database."-shaped field — but they are the +// module's contract with a future validator, so they are pinned here rather than left as +// untested code that only looks safe. +func TestDbSectionQualifyContract(t *testing.T) { + named := namedDatabaseSection("reporting") + + t.Run("root_returns_the_error_untouched", func(t *testing.T) { + original := NewInvalidFieldError("database.host", "bad", nil) + + got := rootDatabaseSection().qualify(original) + + assert.Same(t, original, got, "the root section has nothing to add") + }) + + t.Run("non_config_error_keeps_the_path_in_the_message", func(t *testing.T) { + got := named.qualify(errors.New("boom")) + + var cfgErr *ConfigError + assert.False(t, errors.As(got, &cfgErr)) + assert.ErrorContains(t, got, "databases.reporting: boom") + }) + + t.Run("original_error_is_left_alone", func(t *testing.T) { + original := NewInvalidFieldError("database.host", "bad", nil) + + got := named.qualify(original) + + var cfgErr *ConfigError + require.ErrorAs(t, got, &cfgErr) + assert.Equal(t, "databases.reporting.host", cfgErr.Field) + assert.Equal(t, "database.host", original.Field, + "the connect door shares these constructors and must not inherit a section path") + }) + + t.Run("field_shapes", func(t *testing.T) { + tests := []struct { + name string + field string + want string + }{ + {name: "key_under_database", field: "database.host", want: "databases.reporting.host"}, + {name: "nested_key", field: "database.pool.max.connections", want: "databases.reporting.pool.max.connections"}, + {name: "bare_database", field: fieldDatabase, want: "databases.reporting"}, + {name: "empty", field: "", want: "databases.reporting"}, + {name: "not_key_shaped", field: "oracle connection identifier", want: "databases.reporting.oracle connection identifier"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, named.qualifyField(tt.field)) + }) + } + }) +} + +func TestNormalizeDatabaseValuesConnectKeepsRootSpelling(t *testing.T) { + cfg := DatabaseConfig{ + Type: PostgreSQL, Host: "h", Port: 5432, Database: "d", Username: "u", + TLS: TLSConfig{CertFile: "c.pem"}, + } + + err := normalizeDatabaseValues(&cfg, dbStrictnessConnect) + + var cfgErr *ConfigError + require.ErrorAs(t, err, &cfgErr) + assert.Equal(t, "database.tls", cfgErr.Field) +} + func TestNormalizeDatabaseSectionRootAbsentLeavesConfigUntouched(t *testing.T) { cfg := DatabaseConfig{} require.NoError(t, normalizeDatabaseSection(&cfg, rootDatabaseSection())) diff --git a/wiki/adr_076_section_qualified_config_error_field.md b/wiki/adr_076_section_qualified_config_error_field.md new file mode 100644 index 00000000..e45a2978 --- /dev/null +++ b/wiki/adr_076_section_qualified_config_error_field.md @@ -0,0 +1,100 @@ +# ADR-076: A database section's errors are addressed to that section + +- **Status**: Accepted +- **Date**: 2026-08-20 +- **Related**: [ADR-047](adr_047_database_absence_vs_misconfiguration.md) (absence vs misconfiguration, which is what the root section's placement rule encodes) · [ADR-051](adr_051_delivered_empty_database_identity.md) (the delivered-empty check, whose key spelling this now matches) + +## Context + +A deployment can carry several database sections: the root `database`, any number +of `databases.`, and — under multitenancy — `multitenant.tenants..database`. +They share one normalization module, and therefore one set of error constructors, +all of which name their fields in the root spelling: `database.host`, +`database.type`, `database.tls`. + +At the startup door the section path was attached by wrapping — +`fmt.Errorf("%s: %w", section.path, err)` — so the rendered message read +`databases.reporting: config_missing: database.database required`. The path was in +the text, but the `*ConfigError` a consumer reaches through `errors.As` still said +`Field = "database.database"`. A consumer that switches on `Field` — to point an +operator at the offending key, to decide whether a tenant is safe to skip — could +not tell which section failed, and would act on the root section's name for a +failure in someone else's. + +That also put the module at odds with its neighbour: ADR-051's delivered-empty check +already emits fully path-qualified keys for exactly these sections +(`databases.reporting.host`), and `NewNamedDatabaseError` already addresses a named +section by name. Two spellings for the same key, from the same package. + +## Decision + +The section path is carried by `Field`, and only by `Field`. + +`normalizeDatabaseSection` — the startup door, the one place that knows both the +section path and its placement — rewrites the error it gets back before returning +it. A key under the root section swaps its `database` head for the section path, so +`database.host` becomes `databases.reporting.host` and, for a tenant, the section's +own trailing `.database` is preserved: `multitenant.tenants.acme.database.host`. +That is byte-identical to what the delivered-empty check emits for the same key. + +A field that is not key-shaped is prefixed rather than rewritten. The Oracle +connection-identifier check names one (`oracle connection identifier`), and prefixing +keeps the offending name — `databases.reporting.oracle connection identifier` — where +a strict rewrite would have to drop it. + +The rewrite works on a copy. The constructors are shared with the connect door +(`dbStrictnessConnect`), which resolves a config with no section path to speak of, so +mutating the original would leak a path into errors that have no section. The root +section is returned untouched, and the wrapping message is gone: the path in `Field` +is the one copy. + +## Consequences + +- A consumer matching `ConfigError.Field` against a literal now sees the qualified + spelling for non-root sections. `errors.As` plus `strings.HasSuffix(field, ".host")` + survives the change; `field == "database.host"` does not. +- Root-section errors are unchanged, in both `Field` and rendered text. +- The rendered message for a non-root section loses its `databases.reporting: ` + prefix and gains the path inside the field instead. Log greps that pinned the old + prefix need repointing; the path is still there, spelled once. +- `Action` still carries the root env-var hint (`set DATABASE_DATABASE env var …`) + even for a named section, whose real variable is `DATABASES_REPORTING_DATABASE`. Left + alone deliberately — rewriting hint text is a different problem from addressing an + error — but the deferral has a cost worth stating: the error is now internally + inconsistent, `Field` naming one section while `Action` names the root's variable. + Before this change the wrapping prefix framed the whole error as the section's, so the + root-spelled hint read as a template. Tracked as #1114. +- The RUNTIME door stays root-spelled, so the codebase is qualified at startup only. A + dynamic `DBConfigProvider` tenant resolved through `database.DbManager` reports + `Field = "database.tls"` with the tenant key in the wrapping message, where the same + tenant declared statically now reports `multitenant.tenants.acme.database.tls`. A + consumer routing on `strings.HasPrefix(field, "multitenant.tenants.")` therefore fires + for static tenants and never for dynamic ones. That is the connect door's asymmetry + (ADR-050), which this ADR does not touch; it is tracked as #1113, together with the + three other spellings the tenant tree still uses for sibling failures. + +Migration: [C60.16](migrations.md). + +## Alternatives considered + +**Thread the section path into every error constructor.** It would put the path at +the point each error is raised, which reads well — and it changes every call site in +the package for a property only two of the three doors have, then has to answer what +the connect door passes. The door that knows the section is the door that should say +so. + +**Give `normalizeDatabaseValues` the section, so its errors are born addressed.** The +middle option between the two above: one parameter at one seam, no `errors.As`, no +prefix surgery, and `Action` would be built from the right head for free. It is the +better shape and it is deliberately not taken here, because it changes the signature +the connect door shares — the door that has no section — and this change is meant to be +readable as an addressing fix, not a re-plumbing of the module. If the runtime door is +ever qualified too, this is where to start. + +**Keep the wrapper and leave `Field` alone.** The status quo: the path is in the +message, so a human reading logs is fine. It fails the consumer reading the typed +error, which is the audience `ConfigError` exists for. + +**Rewrite the Oracle outlier into a key path at the same time.** Tempting, and out of +scope here: renaming a field is a separate behaviour change for anyone matching on it, +and it would ride in unannounced under an addressing change. diff --git a/wiki/architecture_decisions.md b/wiki/architecture_decisions.md index bea716a8..2f1852af 100644 --- a/wiki/architecture_decisions.md +++ b/wiki/architecture_decisions.md @@ -1394,6 +1394,25 @@ unexports eight debug response types with their JSON unchanged. See --- +### [ADR-076: A Database Section's Errors Are Addressed to That Section](adr_076_section_qualified_config_error_field.md) + +**Date:** 2026-08-20 | **Status:** Accepted + +Root, `databases.`, and `multitenant.tenants..database` share one normalization module +and therefore one set of error constructors, all spelling their fields against the root +(`database.host`). The startup door used to attach the section path by WRAPPING, so the message +read `databases.reporting: … database.database required` while the `*ConfigError` behind +`errors.As` still said `Field = "database.database"` — a consumer switching on `Field` could not +tell which section failed, and the spelling disagreed with the path-qualified keys +`UntypedDatabaseSections` and ADR-051's delivered-empty check already emit. The path now lives in +`Field` and nowhere else: `databases.reporting.host`, `multitenant.tenants.acme.database.host`, +rewritten on a copy so the connect door — which has no section — keeps the root spelling. + +**Key Benefits:** one spelling for one key across the package; the typed error names the section. +**Watch:** `field == "database.host"` matchers break for non-root sections (`errors.As` + +`strings.HasSuffix` survives), and the message loses its `databases.reporting: ` prefix. See +`[C60.16]` in [migrations.md](migrations.md). + ### [ADR-075: One Normalized Default per Scheduler Timeout Key](adr_075_scheduler_timeout_single_default.md) **Date:** 2026-08-20 | **Status:** Accepted @@ -1510,7 +1529,7 @@ deliberately unchanged: a consume span is still a root span. See [migrations.md] ### Numbering Policy -ADR numbers (ADR-001 through ADR-075) reflect **decision/adoption sequence**, not strict chronological order. The authoritative timeline for each decision is the date in its individual ADR header (e.g., ADR-008 is dated 2025-01-10 while ADR-011 is dated 2025-11-09). When reviewing historical chronology, sort by the dates in the ADR index rather than by number. For example, [ADR-011](adr_011_redis_cache.md) introduced the `ModuleDeps` Cache extension — a breaking API change — and its number simply indicates it was the eleventh decision adopted, not that it followed ADR-010 temporally. +ADR numbers (ADR-001 through ADR-076) reflect **decision/adoption sequence**, not strict chronological order. The authoritative timeline for each decision is the date in its individual ADR header (e.g., ADR-008 is dated 2025-01-10 while ADR-011 is dated 2025-11-09). When reviewing historical chronology, sort by the dates in the ADR index rather than by number. For example, [ADR-011](adr_011_redis_cache.md) introduced the `ModuleDeps` Cache extension — a breaking API change — and its number simply indicates it was the eleventh decision adopted, not that it followed ADR-010 temporally. ## Writing New ADRs diff --git a/wiki/migrations.md b/wiki/migrations.md index 98ccd302..048e4d85 100644 --- a/wiki/migrations.md +++ b/wiki/migrations.md @@ -45,7 +45,7 @@ v0.39.1 ─E40─ v0.40.0 ─E401─ v0.40.1 ─E41─ v0.41.0 ─E42─ v0.42.0 | E58 | v0.57.0 → v0.58.0 | compile-break + breaking (C58.3 aborts startup) + behavior (C58.4, C58.5) | 5 | C58.1 C58.2 C58.3 | check every environment for a **negative** `cache.manager.maxsize` or `cache.manager.idlettl`, and — in multi-tenant mode with `cache.manager.maxsize` unset — a negative `multitenant.limits.tenants`, which becomes the pool size. Under `cache.enabled: false` such a value used to be inert and now aborts startup (C58.3); and if any dashboard, alert, or saved query reads OTLP-exported log records by a `service.*`, `telemetry.sdk.*`, or `deployment.environment.name` **record** attribute your code sets as a log field, re-key it to the `app.`-prefixed name (C58.4); and audit the log backend for dashboards, alerts, or saved queries filtering log records by **any** record-level resource attribute — the framework's `service.*` / `telemetry.sdk.*` / `deployment.environment.name` plus every key your deployment injects via `OTEL_RESOURCE_ATTRIBUTES` (`k8s.pod.name`, …) — since all of them move to resource level only; no code grep finds these (C58.5) | | E581 | v0.58.0 → v0.58.1 | silent-behavior | 3 | none | if you cache any type carrying a time.Time, decide before the bump whether a compare-and-set on a sub-second timestamp may fail during the rolling deploy (C581.1); and if `observability.logs.samplingrate` is set to any value strictly between 0.0 and 1.0, expect the exported INFO/DEBUG log volume AND the membership of the sampled set to change: a rate at or above 0.00005 and below 0.01 exported nothing before the bump and starts exporting its configured fraction after it, a rate that is not a whole percent stops flooring (0.999 was 99%, now 99.9%), and every fractional rate redraws which traces land in the sample; a rate below 0.00005, plus 0.0 and 1.0, are unaffected (C581.2); and if you call `Close()` directly on a `DbManager`/`CacheManager`/`messaging.Manager`, know that a handle still borrowed by in-flight work now stays open until its final release instead of closing immediately (C581.3) | | E59 | v0.58.1 → v0.59.0 | compile-break (C59.2, C59.3, C59.13) + silent-behavior (C59.1, C59.4, C59.14 turns a hand-built config's silently-off secret floor into a startup abort, C59.9 lets a valid Oracle case or whitespace variant build and write, while other newly-accepted spellings still fail later) + breaking (C59.5 rejects a password, C59.6 rejects a dynamic config's TLS material, C59.7 rejects an upsert call, C59.8 rejects another, C59.10 rejects a duplicated conflict column, C59.11 rejects a `database.tls` shape at startup, C59.12 rejects a hand-built config at construction) | 14 | C59.2 C59.3 C59.13 (only partially — a hand-built config that never set the field still compiles; see C59.14) | if your service sits behind a proxy on a **public** address (CloudFront, a partner edge), set `server.trustedproxies` to its CIDR range before the bump — otherwise that proxy is itself returned as the client and every caller behind it collapses into a single rate-limit bucket; and check the load balancer for any mode that writes a non-IP `X-Forwarded-For` entry — on AWS ALB that is `routing.http.xff_client_port.enabled` (appends `client_ip:port`) and, separately, `routing.http.xff_header_processing.mode = remove` — since either keys the entire fleet on the load balancer's own address after the bump and `server.trustedproxies` cannot fix it; the remedy is deployment-side (C59.1); and if any of your code — **including test files**, which `go build` does not compile — implements `cache.Cache`, add the new `CompareAndDelete` method, and before swapping a lock's `Delete` release for it make sure the lock is acquired with a **positive** TTL, since a `ttl == 0` lock that a token-verified release declines to remove is held forever (C59.3); and grep your **test** files for a `cache/testing.MockCache` handed a context that is already canceled or expired while the call is expected to succeed — the mock's cancellation check no longer depends on a configured `WithDelay`, so that call now returns the context's error (C59.4); and if you call `ProvisionPGRoles` or `PGRoleProvisioningSQL` with a `PGRoleSpec` whose `MigratorPassword` or `RuntimePassword` is read from a file, a mounted secret, an environment read, or a command substitution, `strings.TrimSpace` it before the bump — a password containing CR, LF, or NUL is now rejected by `Validate` instead of provisioning, and any credential whose provisioning failure was logged while its password contained a newline should be rotated, since the first line of that secret reached the error string (C59.5); and hand-read every `BuildUpsert` call for a column key present in BOTH `conflictColumns` and `updateColumns` — grep finds the calls but not the overlap, since both maps are usually built dynamically — because on PostgreSQL such a call built and ran before the bump and now returns an error from the builder, while on Oracle it already failed and now fails earlier, at build time with the builder's message rather than at execution with ORA-38104; match keys the way each vendor does, since Oracle folds the unquoted identifiers it emits to upper case (so `id` and `ID` are one column there, and are now rejected) while PostgreSQL quotes every identifier and keeps them distinct; then compare that column's update value against its insert value before remediating — equal means dropping it from `updateColumns` changes no column value — though if it is the column's **only** entry the set empties, which builds `DO NOTHING` on PostgreSQL and drops Oracle's `WHEN MATCHED` arm, so a matched row stops being updated at all, its UPDATE triggers stop firing and `RETURNING` yields no row; keep a real non-conflict column or issue an explicit `UPDATE` — under the same transaction and locking rule as below — where that matters — but differing means the call was rewriting the conflict column on a matched row, which no vendor-portable upsert can express, so those need a separate `UPDATE` rather than a dropped column — run it in the same transaction as the insert, keyed on the conflict columns, and holding the row lock the single statement took for you (`SELECT … FOR UPDATE` or equivalent), because splitting one atomic upsert into two statements lets a concurrent writer interleave and under READ COMMITTED a shared transaction alone does not stop it (C59.7); and if a dynamic multi-tenant `DBConfigProvider` returns a `database.connectionstring` with no `type`, that tenant now dials a real database at first use instead of failing every request with `unsupported database type: ""`, and if you supply `Options.DatabaseConnector` it now receives an inferred `type` for a recognized scheme instead of an empty one — inference is unconditional, since that option's exemption only ever covered the startup guard; then enumerate the same source for any tenant carrying `database.tls.cert`/`database.tls.key`/`database.tls.ca` next to an Oracle type or `oracle://` DSN, or exactly one of `database.tls.cert`/`database.tls.key` on a PostgreSQL one, since those connect today with the TLS material silently dropped and stop connecting after the bump, typed configs included (C59.6); and hand-read the `BuildUpsert` shortlist again for a conflict column that names no column of `insertColumns` by vendor identity — Oracle already rejected those, PostgreSQL let the column fall to its table default, which was inert where the default was absent but is a working pattern where it is a sequence, a `current_setting(...)` or a generated column, and after the bump both vendors refuse it, so a sequence or `current_setting(...)` default means passing the value in `insertColumns` while a generated column — which PostgreSQL forbids writing directly — means `database.Raw` or a schema change; drop any match on the precondition texts too, since `conflict columns required for Oracle MERGE` and `conflict columns required for PostgreSQL upsert` both become `conflict columns required for upsert` (C59.8); and check whether any `BuildUpsert` call can pass the same column twice in `conflictColumns` — duplicates are judged by vendor identity, so an exact repeat on either vendor, or on Oracle a case variant of a non-reserved identifier, which Oracle emits unquoted and folds (its reserved words are quoted and stay case-sensitive, so `["level", "LEVEL"]` is still accepted; on PostgreSQL no case variant is a duplicate) — because both vendors now refuse it where PostgreSQL previously failed only at execution (`42P10`) and Oracle accepted it outright, so de-duplicate at the call site by the vendor's own rules rather than by lower-casing, which is wrong on PostgreSQL (C59.10); and read that shortlist once more for an Oracle call whose conflict column and insert key spell the same column differently — a case variant or a whitespace-padded key — since those were refused at build time and now build and write, with no signal after the bump, so any validation you were getting from that rejection has to move into your own code before it (C59.9); and grep every environment for a `database.tls` block, because four shapes that booted green now abort startup — static configuration at boot, while a dynamic `DBConfigProvider` record with the same shape boots green and fails at first connection acquisition, so check dynamic tenants by acquiring a connection: a PostgreSQL `mode` outside the sslmode allowlist, `cert`/`key`/`ca` under an empty/`disable`/`allow`/`prefer` mode (pgx was discarding the material, allowing a plaintext downgrade, or — for `ca: system` — silently upgrading the mode to `verify-full`; an unset mode defaults to `prefer`, so this is the common case), any `database.tls.*` alongside a `connectionstring`, and — extending C42.1, which said mode alone still passed — `database.tls.mode` on Oracle; setting `verify-ca`/`verify-full` — or `require` plus a `ca` — also makes that connection verify the server for the first time, so confirm the CA chain before the bump, while bare `require` encrypts without authenticating the server (C59.11); and if any code hands `app.NewWithConfig` or a direct `app.Builder` chain a hand-built config, run it against `config.Validate`'s rules before the bump — missing `app.name`/`app.version`, zero server timeouts and invalid vendors now fail construction (C59.12); and if any Go code sets `config.KeyStoreConfig.SecretMinLength`, write `new(0)` / `new(N)` — the field is now `*int` (C59.13); and if a hand-built config with symmetric secrets never set it, it relied on the Go zero silently disabling the floor and now gets 32 bytes, so any secret shorter than that fails startup — grep finds nothing for this shape, read C59.14 | -| E60 | v0.59.0 → v0.60.0 | compile-break (C60.4 — internal helpers nothing outside app/ used) + breaking (C60.1 fails a migrate run; C60.11 rejects an upsert call whose column keys name one Oracle column twice, that names an Oracle conflict column it also updates under another spelling, or that Oracle's MERGE cannot name — and, on BOTH vendors, one whose key carries an unescaped interior quote; C60.12 rejects a negative scheduler timeout and a module Init with no config; C60.11 rejects an upsert call whose column keys name one Oracle column twice, or that Oracle's MERGE cannot name) + silent-behavior (C60.2 forwards `database.tls` to Flyway, where nothing reached it before; C60.3 changes strings on `/ready`'s 200 body — including the `db_stats` → `database_stats` rename — and folds two entries out of the debug health view; no status code moves; C60.5 — idle-cleanup sweeps start at manager construction; five startup/shutdown log lines retire; C60.7 — the AMQP failure and panic lines' second `correlation_id` stamp becomes `amqp_correlation_id`; C60.8 — inbound trace identifiers failing validation are discarded and regenerated; C60.9 — the streams lane's consume telemetry and log lines change shape; C60.10 — a published message's `CorrelationId` carries the same id as its own `X-Request-ID` header, or nothing at all where that id fails validation) + compile-break (C60.6 — `messaging.StartConsumeSpan` removed; the consumed-messages counter now counts at completion with `error.type`) | 12 | C60.4; C60.6 (only partially — the code door is compiler-caught; the telemetry door is silent, see its gate) | if you run the `go-bricks-migrate` CLI, resolve every tenant it reads before the bump — `go-bricks-migrate info` per tenant is the cheap check — because configs it accepted are now validated the way the framework validates them before dialing, which covers the `database.tls` shapes of [C59.11] plus a missing Oracle connection identifier and an unloadable `database.timezone`; the per-tenant AWS Secrets Manager payloads cannot be grepped from your repo, so enumerate the prefix in each account rather than trusting a config-file sweep (C60.1); and grep dashboards, alerts, synthetic checks and contract tests for `db_stats`, `not_ready`, `connection_failed`, `no_active_connections`, `database_manager`/`messaging_manager`, for `messaging_stats`/`cache_stats` pinned to `{}` on a service without that kind, and for `overall_status == unknown` on `/_sys/health-debug` — every kind now speaks one vocabulary (`healthy`, `unhealthy`, `not_configured`, `disabled`, `per_tenant`), a disabled kind's stats read `{"status":"disabled"}`, messaging and cache read `per_tenant` in multi-tenant deployments where they read `not_configured`, and the debug view lists every classic kind; the 200 body's `db_stats` key is now `database_stats`, the debug view's `database_manager`/`messaging_manager` entries are gone (their statistics are the `database`/`messaging` entries' details), and `overall_status` reads `degraded` where it read `unknown` for a non-critical kind that is not live; a critical kind that is down still answers 503 exactly as before (C60.3); and grep your Go code — test files included — for the sixteen removed app helpers and the eight unexported debug response types (C60.4). And grep log-based alerts and saved queries for the five retired cleanup-loop lines (`Starting/Stopping database manager cleanup loop`, `Starting/Stopping messaging manager cleanup loop`, `Manager cleanup loops stopped`) — they have no renamed equivalent (C60.5). And grep your Go code for `StartConsumeSpan` and your dashboards for `messaging.client.consumed.messages` — the call is a build break, the counter now increments at completion and carries `error.type` on failure (C60.6). And grep log-based alerts, saved queries and log-parsing tests for `correlation_id` on the AMQP consumer's failure and panic lines — the delivery's own AMQP `CorrelationId` is stamped as `amqp_correlation_id` there now, and `correlation_id` carries only the framework trace ID (C60.7). And search your log backend for `correlation_id` values that are empty, longer than 128 characters, or contain anything outside `A-Za-z0-9_-` — those identifiers are now discarded at ingress and replaced by a framework-minted one, so correlation with the upstream that emitted them breaks; that query covers the X-Request-ID half only, so also check the gateway emitting your `traceparent`/`tracestate`, which are now validated too and show up in no log field (C60.8). And if you consume native streams, grep dashboards and log queries for the streams consumer's failure and panic lines and for its span attributes — the lane now runs on the shared delivery pipeline, so its lines gain the spine, its panic line's wording changes, and its span gains the four shared attributes (C60.9). And if anything downstream reads a message's AMQP `CorrelationId` property rather than its `X-Request-ID` header, that property now carries the same aligned trace id the header carries — a 32-hex traceparent trace-id where an HTTP-originated publish used to put the request's UUID (C60.10). And if you build `config.Config` in Go or call `scheduler.Module.Init` / `app.NewModuleRegistry` yourself, check both `scheduler.timeout.*` keys: zero now normalizes (30s/25s, where the module used to fall back to 30s for slowjob), a negative value fails validation, and a module handed no config fails Init (C60.12). And if you build Oracle upserts, grep `BuildUpsert` call sites for column maps assembled dynamically — two keys that fold to one Oracle column, a conflict column spelled differently from the insert or update key naming that same column, and dotted or function-shaped keys, are now build-time errors rather than SQL Oracle rejects at parse or at execution (C60.11); on PostgreSQL the only new rejection is a key carrying a quote that is not doubled, which used to leave the identifier and become SQL, and no identity rule changes there. | +| E60 | v0.59.0 → v0.60.0 | compile-break (C60.4 — internal helpers nothing outside app/ used) + breaking (C60.1 fails a migrate run; C60.16 re-addresses a non-root database section's ConfigError.Field; C60.11 rejects an upsert call whose column keys name one Oracle column twice, that names an Oracle conflict column it also updates under another spelling, or that Oracle's MERGE cannot name — and, on BOTH vendors, one whose key carries an unescaped interior quote; C60.12 rejects a negative scheduler timeout and a module Init with no config; C60.11 rejects an upsert call whose column keys name one Oracle column twice, or that Oracle's MERGE cannot name) + silent-behavior (C60.2 forwards `database.tls` to Flyway, where nothing reached it before; C60.3 changes strings on `/ready`'s 200 body — including the `db_stats` → `database_stats` rename — and folds two entries out of the debug health view; no status code moves; C60.5 — idle-cleanup sweeps start at manager construction; five startup/shutdown log lines retire; C60.7 — the AMQP failure and panic lines' second `correlation_id` stamp becomes `amqp_correlation_id`; C60.8 — inbound trace identifiers failing validation are discarded and regenerated; C60.9 — the streams lane's consume telemetry and log lines change shape; C60.10 — a published message's `CorrelationId` carries the same id as its own `X-Request-ID` header, or nothing at all where that id fails validation) + compile-break (C60.6 — `messaging.StartConsumeSpan` removed; the consumed-messages counter now counts at completion with `error.type`) | 13 | C60.4; C60.6 (only partially — the code door is compiler-caught; the telemetry door is silent, see its gate) | if you run the `go-bricks-migrate` CLI, resolve every tenant it reads before the bump — `go-bricks-migrate info` per tenant is the cheap check — because configs it accepted are now validated the way the framework validates them before dialing, which covers the `database.tls` shapes of [C59.11] plus a missing Oracle connection identifier and an unloadable `database.timezone`; the per-tenant AWS Secrets Manager payloads cannot be grepped from your repo, so enumerate the prefix in each account rather than trusting a config-file sweep (C60.1); and grep dashboards, alerts, synthetic checks and contract tests for `db_stats`, `not_ready`, `connection_failed`, `no_active_connections`, `database_manager`/`messaging_manager`, for `messaging_stats`/`cache_stats` pinned to `{}` on a service without that kind, and for `overall_status == unknown` on `/_sys/health-debug` — every kind now speaks one vocabulary (`healthy`, `unhealthy`, `not_configured`, `disabled`, `per_tenant`), a disabled kind's stats read `{"status":"disabled"}`, messaging and cache read `per_tenant` in multi-tenant deployments where they read `not_configured`, and the debug view lists every classic kind; the 200 body's `db_stats` key is now `database_stats`, the debug view's `database_manager`/`messaging_manager` entries are gone (their statistics are the `database`/`messaging` entries' details), and `overall_status` reads `degraded` where it read `unknown` for a non-critical kind that is not live; a critical kind that is down still answers 503 exactly as before (C60.3); and grep your Go code — test files included — for the sixteen removed app helpers and the eight unexported debug response types (C60.4). And grep log-based alerts and saved queries for the five retired cleanup-loop lines (`Starting/Stopping database manager cleanup loop`, `Starting/Stopping messaging manager cleanup loop`, `Manager cleanup loops stopped`) — they have no renamed equivalent (C60.5). And grep your Go code for `StartConsumeSpan` and your dashboards for `messaging.client.consumed.messages` — the call is a build break, the counter now increments at completion and carries `error.type` on failure (C60.6). And grep log-based alerts, saved queries and log-parsing tests for `correlation_id` on the AMQP consumer's failure and panic lines — the delivery's own AMQP `CorrelationId` is stamped as `amqp_correlation_id` there now, and `correlation_id` carries only the framework trace ID (C60.7). And search your log backend for `correlation_id` values that are empty, longer than 128 characters, or contain anything outside `A-Za-z0-9_-` — those identifiers are now discarded at ingress and replaced by a framework-minted one, so correlation with the upstream that emitted them breaks; that query covers the X-Request-ID half only, so also check the gateway emitting your `traceparent`/`tracestate`, which are now validated too and show up in no log field (C60.8). And if you consume native streams, grep dashboards and log queries for the streams consumer's failure and panic lines and for its span attributes — the lane now runs on the shared delivery pipeline, so its lines gain the spine, its panic line's wording changes, and its span gains the four shared attributes (C60.9). And if anything downstream reads a message's AMQP `CorrelationId` property rather than its `X-Request-ID` header, that property now carries the same aligned trace id the header carries — a 32-hex traceparent trace-id where an HTTP-originated publish used to put the request's UUID (C60.10). And if you build `config.Config` in Go or call `scheduler.Module.Init` / `app.NewModuleRegistry` yourself, check both `scheduler.timeout.*` keys: zero now normalizes (30s/25s, where the module used to fall back to 30s for slowjob), a negative value fails validation, and a module handed no config fails Init (C60.12). And if you build Oracle upserts, grep `BuildUpsert` call sites for column maps assembled dynamically — two keys that fold to one Oracle column, a conflict column spelled differently from the insert or update key naming that same column, and dotted or function-shaped keys, are now build-time errors rather than SQL Oracle rejects at parse or at execution (C60.11); on PostgreSQL the only new rejection is a key carrying a quote that is not doubled, which used to leave the identifier and become SQL, and no identity rule changes there. And grep your Go code for `ConfigError.Field` compared to a literal `database.*` key: a non-root database section now reports `databases..host` / `multitenant.tenants..database.host` there instead of the root spelling, and its message loses the `databases.: ` prefix (C60.16). | **4 — Read each atom's gate before acting.** Every atom carries `when: match | no-match | always`: @@ -3228,7 +3228,9 @@ None of them is exhaustive — all three are line-oriented and blind to an impor sites whose column maps are assembled dynamically, including any conflict column spelled differently from the insert or update key naming the same column; and on PostgreSQL, where nothing else changes, re-read those same call sites for a key containing a quote that is not - doubled (C60.11) + doubled (C60.11); and if any code compares `config.ConfigError.Field` to a + literal `database.*` key, switch it to a suffix match — a non-root section now names itself + in that field (C60.16) - exit: `go get github.com/gaborage/go-bricks@v0.60.0 && go mod tidy && go build ./... && go test ./...` ### [C60.1] `go-bricks-migrate` validates every resolved database config · breaking · when: match @@ -3808,6 +3810,43 @@ None of them is exhaustive — all three are line-oriented and blind to an impor (`normalizeScheduler`) · `scheduler/module.go` (`Init`, `Shutdown`, `determineJobSeverity`) +### [C60.16] a database section's `ConfigError.Field` names that section · breaking · when: match + +- detect: `git grep -nE 'ConfigError|errors\.As' -- '*.go'` in your own code, then read every + hit that compares `.Field` to a literal starting `database.` — those are the matchers this + changes. Log-side: grep dashboards and saved queries for the message prefix + `databases.: ` or `multitenant.tenants..database: `. +- scope: normalization errors from a NON-ROOT database section used to carry the section path + in the wrapping message while the `*ConfigError` behind `errors.As` kept the root spelling — + message `databases.reporting: config_missing: database.database required`, but + `Field == "database.database"`. The path now lives in `Field` and only there: + `databases.reporting.database`, `databases.reporting.host`, + `multitenant.tenants.acme.database.host`. That spelling is byte-identical to the keys ADR-051's + delivered-empty check already emits for the same section, so the package now has one + spelling per key. A field that is not key-shaped is PREFIXED rather than + rewritten, keeping the name: `databases.reporting.oracle connection identifier`. Unchanged: + the root section (`database.host`, same `Field`, same text), the connect door — a + `DBConfigProvider` resolving a tenant at RUNTIME still reports the root spelling + (`database.tls`), with the tenant key only in the wrapping message — and `Action`, which + still names the ROOT env var (`set DATABASE_DATABASE env var …`) even for a named section, + whose real variable is `DATABASES_REPORTING_DATABASE`. +- gate: match = your code reads `ConfigError.Field`, or a log query pins the old message + prefix. no-match = you only read the rendered error as text and grep for the key, which is + still in it. +- apply: replace `field == "database.host"` with a suffix match — + `errors.As(err, &cfgErr) && strings.HasSuffix(cfgErr.Field, ".host")` — which matches root + and section spellings alike; use `strings.HasPrefix(cfgErr.Field, "databases.")` when you + need to know it was a named section. Repoint log queries from the `databases.: ` + prefix to the qualified key. If you route per-tenant failures on that prefix, know it + matches STATIC tenants only: a dynamic `DBConfigProvider` tenant still reports the root + spelling, so keep whatever handling you have for that path. +- verify: give a `databases.` entry a real host and no database name, boot, and read the + startup error: it names `databases..database` once. Before this change the same boot + printed the path in the prefix and `database.database` in the field. +- ref: [ADR-076](adr_076_section_qualified_config_error_field.md) · + [ADR-051](adr_051_delivered_empty_database_identity.md) · + `config/database_section.go` (`dbSection.qualify`, `qualifyField`) + --- *The sections below are reference material: the two config-key rename lookup tables (linked from atoms C401.1 and C41.7), followed by pre-v0.39 changes retained for consumers upgrading from older releases.* From f749c7ce8d56c729cd2e01c84363c2c4456445ec Mon Sep 17 00:00:00 2001 From: Gabriel Rosales Date: Thu, 20 Aug 2026 19:34:52 -0600 Subject: [PATCH 2/2] docs(config): scope the C60.16 matcher to the database families CodeRabbit (mirror#6). The migration advice told consumers to replace field == "database.host" with strings.HasSuffix(field, ".host"), but ConfigError.Field is not a database-only namespace: cache.redis.host and messaging.broker.host both end in .host, so a consumer following the runbook would route an unrelated config error as a database-host error. C60.16 now carries a predicate scoped to the three database field families -- exact match on database., and a suffix only behind the databases. or multitenant.tenants. prefix -- and the ADR, the ADR index summary and the CLAUDE.md line say the same thing rather than "match by suffix". --- CLAUDE.md | 2 +- ...76_section_qualified_config_error_field.md | 7 ++++-- wiki/architecture_decisions.md | 6 +++-- wiki/migrations.md | 24 ++++++++++++++----- 4 files changed, 28 insertions(+), 11 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 8f446e45..7e29a7e3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -424,7 +424,7 @@ GoBricks breaks its own API surface when justified. Greenfield work uses the new - **Dead app lifecycle surface removed (ADR-067):** `MessagingInitializer` and `ConnectionPreWarmer` (constructors and methods included), `Options.Database` and `Options.MessagingClient` are gone; the eight debug response types are unexported with their JSON unchanged. - **One delivery pipeline (ADR-068):** `messaging.StartConsumeSpan` is removed — a service driving its own consume loop starts its own span — and the AMQP `messaging.client.consumed.messages` counter is recorded at completion with `error.type` instead of at receive without it. - **Scheduler timeouts normalize (ADR-075):** `scheduler.timeout.shutdown`/`slowjob` default in `config.Validate` (30s/25s — hand-built configs move 30s → 25s), negatives fail validation, and `scheduler.Module.Init` requires a NORMALIZED `deps.Config` — non-nil, both timeouts positive — so a config assembled outside app construction must go through `config.Validate` first. -- **Section-qualified config errors (ADR-076):** a non-root database section's `ConfigError.Field` names that section (`databases.reporting.host`, `multitenant.tenants.acme.database.host`); match by suffix, not equality. Root and the connect door keep the root spelling. +- **Section-qualified config errors (ADR-076):** a non-root database section's `ConfigError.Field` names that section (`databases.reporting.host`, `multitenant.tenants.acme.database.host`); match with a database-scoped predicate, not equality and not a bare suffix — `cache.redis.host` ends in `.host` too. Root and the connect door keep the root spelling. ## File Organization diff --git a/wiki/adr_076_section_qualified_config_error_field.md b/wiki/adr_076_section_qualified_config_error_field.md index e45a2978..7c9b5601 100644 --- a/wiki/adr_076_section_qualified_config_error_field.md +++ b/wiki/adr_076_section_qualified_config_error_field.md @@ -51,8 +51,11 @@ is the one copy. ## Consequences - A consumer matching `ConfigError.Field` against a literal now sees the qualified - spelling for non-root sections. `errors.As` plus `strings.HasSuffix(field, ".host")` - survives the change; `field == "database.host"` does not. + spelling for non-root sections, so `field == "database.host"` stops matching. The + replacement has to be scoped to the database field families rather than a bare suffix + match: `Field` is not a database-only namespace, and `cache.redis.host` ends in `.host` + as well. Match `database.` exactly, and require the `databases.` or + `multitenant.tenants.` prefix before accepting a suffix — C60.16 carries the predicate. - Root-section errors are unchanged, in both `Field` and rendered text. - The rendered message for a non-root section loses its `databases.reporting: ` prefix and gains the path inside the field instead. Log greps that pinned the old diff --git a/wiki/architecture_decisions.md b/wiki/architecture_decisions.md index 2f1852af..a3e58c5f 100644 --- a/wiki/architecture_decisions.md +++ b/wiki/architecture_decisions.md @@ -1409,8 +1409,10 @@ tell which section failed, and the spelling disagreed with the path-qualified ke rewritten on a copy so the connect door — which has no section — keeps the root spelling. **Key Benefits:** one spelling for one key across the package; the typed error names the section. -**Watch:** `field == "database.host"` matchers break for non-root sections (`errors.As` + -`strings.HasSuffix` survives), and the message loses its `databases.reporting: ` prefix. See +**Watch:** `field == "database.host"` matchers break for non-root sections — replace them with +a predicate scoped to the `database.` / `databases.` / `multitenant.tenants.` families, since a +bare suffix match also catches `cache.redis.host` — and the message loses its +`databases.reporting: ` prefix. See `[C60.16]` in [migrations.md](migrations.md). ### [ADR-075: One Normalized Default per Scheduler Timeout Key](adr_075_scheduler_timeout_single_default.md) diff --git a/wiki/migrations.md b/wiki/migrations.md index 048e4d85..7a3f121e 100644 --- a/wiki/migrations.md +++ b/wiki/migrations.md @@ -3229,8 +3229,9 @@ None of them is exhaustive — all three are line-oriented and blind to an impor differently from the insert or update key naming the same column; and on PostgreSQL, where nothing else changes, re-read those same call sites for a key containing a quote that is not doubled (C60.11); and if any code compares `config.ConfigError.Field` to a - literal `database.*` key, switch it to a suffix match — a non-root section now names itself - in that field (C60.16) + literal `database.*` key, switch it to a database-scoped predicate — a non-root section now + names itself in that field, and a bare suffix match would also catch `cache.redis.host` + (C60.16) - exit: `go get github.com/gaborage/go-bricks@v0.60.0 && go mod tidy && go build ./... && go test ./...` ### [C60.1] `go-bricks-migrate` validates every resolved database config · breaking · when: match @@ -3833,10 +3834,21 @@ None of them is exhaustive — all three are line-oriented and blind to an impor - gate: match = your code reads `ConfigError.Field`, or a log query pins the old message prefix. no-match = you only read the rendered error as text and grep for the key, which is still in it. -- apply: replace `field == "database.host"` with a suffix match — - `errors.As(err, &cfgErr) && strings.HasSuffix(cfgErr.Field, ".host")` — which matches root - and section spellings alike; use `strings.HasPrefix(cfgErr.Field, "databases.")` when you - need to know it was a named section. Repoint log queries from the `databases.: ` +- apply: replace `field == "database.host"` with a predicate scoped to the DATABASE field + families — a bare suffix match is wrong, because `ConfigError.Field` is not a + database-only namespace and `cache.redis.host` ends in `.host` too: + + ```go + func isDatabaseField(field, key string) bool { + return field == "database."+key || // root + (strings.HasPrefix(field, "databases.") && strings.HasSuffix(field, "."+key)) || + (strings.HasPrefix(field, "multitenant.tenants.") && + strings.HasSuffix(field, ".database."+key)) + } + ``` + + Then `errors.As(err, &cfgErr) && isDatabaseField(cfgErr.Field, "host")` matches root and + section spellings and nothing else; the two prefixes also tell you WHICH family it was. Repoint log queries from the `databases.: ` prefix to the qualified key. If you route per-tenant failures on that prefix, know it matches STATIC tenants only: a dynamic `DBConfigProvider` tenant still reports the root spelling, so keep whatever handling you have for that path.