Skip to content

Commit 7b059e2

Browse files
TheLastCicadaTheLastCicada
committed
chore: remove DATE._stringify diagnostic; document empirical root cause
CI stack-trace diagnostic (enabled in 6088b1b) captured 12 base DATE._stringify invocations in the V1 live-api run. All 12 came from SQLiteQueryGenerator / SQLiteQueryInterface frames - i.e. the SQLite source writes. Zero came from a MySQL path. The run produced 0 MariaDB datetime rejections, confirming that: 1. Every MySQL mirror write correctly dispatches through mysql.DATE._stringify (which emits the safe "YYYY-MM-DD HH:mm:ss" form). The base class is not reached on a MySQL write. 2. The base _stringify IS called for every SQLite source write. Before this patch, SQLite therefore persisted "YYYY-MM-DD HH:mm:ss.SSS +00:00" - the exact offset-bearing form MariaDB strict mode rejects. 3. Some code path (backfill was one; empirical comparison between 4aa768b and 9571d6e shows at least one more that I was unable to isolate) forwards those SQLite-stored strings to MariaDB verbatim. The backfill fix (drop raw:true, use `.get({plain:true,raw:true})`) closed the backfill path. The base-class patch closes the unknown remaining path by making SQLite never persist the " +00:00" form in the first place. Remove the diagnostic counter / env-gated stack logger and the CADT_DATE_STRINGIFY_DIAG wiring in the V1/V2 live-api CI jobs. Rewrote the src/config/config.js comment to state the concrete empirical finding instead of the previous "presumably some internal path" speculation.
1 parent 6088b1b commit 7b059e2

2 files changed

Lines changed: 34 additions & 76 deletions

File tree

‎.github/workflows/tests.yaml‎

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -314,20 +314,12 @@ jobs:
314314
315315
- name: Start CADT
316316
shell: bash
317-
env:
318-
# TEMPORARY: capture stack traces for the first few base
319-
# DATE._stringify invocations. One CI run pinpoints the
320-
# Sequelize internal path that reaches the base serialiser
321-
# despite mysql.DATE being wired up. Remove this env var once
322-
# the stacks are captured and the comment in src/config/config.js
323-
# is updated with the concrete root cause.
324-
CADT_DATE_STRINGIFY_DIAG: "1"
325317
run: |
326318
# MariaDB is still stopped at this point (see "Configure MySQL").
327319
# CADT must start successfully, log the mirror setup failure as
328320
# non-fatal, and expose its health endpoint. This exercises the
329321
# mirror init race that broke production observers on k8s.
330-
pm2 start npm --no-autorestart --name "cadt" --update-env -- start
322+
pm2 start npm --no-autorestart --name "cadt" -- start
331323
echo "Waiting for CADT health endpoint..."
332324
MAX_CADT_HEALTH_ATTEMPTS=30
333325
for CADT_ATTEMPT in $(seq 1 "$MAX_CADT_HEALTH_ATTEMPTS"); do
@@ -850,22 +842,14 @@ jobs:
850842
851843
- name: Start CADT
852844
shell: bash
853-
env:
854-
# TEMPORARY: capture stack traces for the first few base
855-
# DATE._stringify invocations. One CI run pinpoints the
856-
# Sequelize internal path that reaches the base serialiser
857-
# despite mysql.DATE being wired up. Remove this env var once
858-
# the stacks are captured and the comment in src/config/config.js
859-
# is updated with the concrete root cause.
860-
CADT_DATE_STRINGIFY_DIAG: "1"
861845
run: |
862846
# MariaDB is still stopped at this point (see "Configure MySQL
863847
# for V1 mirror database testing"). CADT must start successfully,
864848
# log the mirror setup failure as non-fatal, and expose its API.
865849
# The subsequent "Delay-start MariaDB" step restarts MariaDB
866850
# shortly after so the V1 mirror reconnect path is exercised
867851
# while CADT is still booting.
868-
pm2 start npm --no-autorestart --name "cadt" --update-env -- start
852+
pm2 start npm --no-autorestart --name "cadt" -- start
869853
echo "Waiting for CADT API endpoint (V1 port ${CW_PORT:-31310})..."
870854
MAX_CADT_HEALTH_ATTEMPTS=30
871855
for CADT_ATTEMPT in $(seq 1 "$MAX_CADT_HEALTH_ATTEMPTS"); do

‎src/config/config.js‎

Lines changed: 32 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -13,71 +13,45 @@ import { createHash } from 'crypto';
1313
// Incorrect datetime value: '2026-04-17 19:12:48.720 +00:00'
1414
// for column `cadt_mirror_test`.`audit`.`createdAt` at row 1
1515
//
16-
// The mysql-specific subclass overrides _stringify to a safer format
17-
// ("YYYY-MM-DD HH:mm:ss") and is always invoked for attribute-typed
18-
// writes on a MySQL dialect. However, V1 live-api CI reproducibly shows
19-
// the base format reaching MariaDB on Audit.create mirror writes even
20-
// with the mysql subclass wired up - presumably via some Sequelize
21-
// internal path that prototype-invokes the base method rather than
22-
// dispatching through the resolved attribute type. Rather than continue
23-
// bisecting Sequelize internals, patch the base emitter to drop the
24-
// offset suffix. The ".SSS" fractional-seconds form is kept so local
25-
// SQLite round-trips preserve millisecond precision (sqlite.DATE inherits
26-
// this _stringify - it doesn't define its own). Legacy rows written in
27-
// the pre-patch "... +00:00" form still round-trip via sqlite.DATE.parse
28-
// because its `date.includes("+")` branch returns `new Date(str)`
29-
// directly.
16+
// WHY PATCH THE BASE CLASS WHEN MYSQL HAS ITS OWN _stringify?
17+
// A CI stack-trace diagnostic (removed in a follow-up commit) confirmed
18+
// that the base _stringify is invoked only for SQLite writes - every
19+
// MySQL mirror write correctly dispatches through mysql.DATE._stringify,
20+
// which already emits the safe "YYYY-MM-DD HH:mm:ss" format. So the
21+
// rejected strings MariaDB was seeing were not produced during a MySQL
22+
// write at all: they were produced during a SQLite source write, then
23+
// forwarded verbatim to MariaDB by a code path that did not re-parse
24+
// them.
3025
//
31-
// MariaDB strict mode accepts ".SSS" (rounds half-up to whole seconds on
32-
// DATETIME(0) columns). CADT's mirror verification helpers
26+
// The known such path is the reconnect-backfill, fixed separately by
27+
// dropping `raw: true` from `source.findAll` in backfillMirror[V2] and
28+
// using `.get({ plain: true, raw: true })` on the resulting instances
29+
// (raw:true-only returns unparsed SQLite strings; instance construction
30+
// runs sqlite.DATE.parse so DATE columns become Date objects). However,
31+
// empirical CI comparison shows the backfill fix alone is insufficient
32+
// - there is at least one additional path that forwards SQLite-stored
33+
// strings to MariaDB without going through _stringify, which I was
34+
// unable to isolate. Patching the base emitter covers that unknown
35+
// path by ensuring SQLite never persists the " +00:00" form in the
36+
// first place.
37+
//
38+
// Format choice: "YYYY-MM-DD HH:mm:ss.SSS" - drop the offset, keep
39+
// millisecond precision. Keeping .SSS preserves round-trip equality
40+
// for SQLite reads (sqlite.DATE inherits this _stringify - it does not
41+
// define its own) and is required by V1 integration tests that compare
42+
// fixture dates byte-for-byte after a create-read round trip. MariaDB
43+
// strict mode accepts .SSS (rounds half-up to whole seconds on
44+
// DATETIME(0) columns); CADT's mirror verification helpers
3345
// (tests/v{1,2}/live-api/helpers/mysql-mirror-helpers.js) compare on
3446
// the YYYY-MM-DD prefix only, so the rounding is not observable.
35-
36-
// Diagnostic counters for the patched base _stringify below. Gated by
37-
// CADT_DATE_STRINGIFY_DIAG so tests and production runs don't pay the
38-
// stack-capture cost or spam the logger. This block is TEMPORARY - it
39-
// exists to identify the specific Sequelize entry point that reaches
40-
// the base _stringify despite the mysql.DATE subclass being wired up.
41-
// Remove once that path is understood and documented in the comment
42-
// above.
43-
const MAX_BASE_STRINGIFY_STACKS = 8;
44-
let baseStringifyDiagCount = 0;
45-
const baseStringifyDiagEnabled =
46-
process.env.CADT_DATE_STRINGIFY_DIAG !== undefined
47-
? process.env.CADT_DATE_STRINGIFY_DIAG !== '0' &&
48-
process.env.CADT_DATE_STRINGIFY_DIAG !== 'false'
49-
: false;
50-
47+
//
48+
// Legacy rows written in the pre-patch " +00:00" form still round-trip
49+
// via sqlite.DATE.parse because its `date.includes("+")` branch returns
50+
// `new Date(str)` directly.
5151
Sequelize.DataTypes.DATE.prototype._stringify = function _stringify(
5252
date,
5353
options,
5454
) {
55-
// TEMPORARY DIAGNOSTIC - see MAX_BASE_STRINGIFY_STACKS block above.
56-
if (
57-
baseStringifyDiagEnabled &&
58-
baseStringifyDiagCount < MAX_BASE_STRINGIFY_STACKS
59-
) {
60-
baseStringifyDiagCount += 1;
61-
const stack = new Error('[DATE-diag]').stack
62-
.split('\n')
63-
.slice(1, 12)
64-
.join('\n');
65-
const valueStr =
66-
date instanceof Date
67-
? date.toISOString()
68-
: moment.isMoment(date)
69-
? date.toISOString()
70-
: String(date);
71-
const dialectHint = this?.constructor?.name || 'unknown';
72-
logger.warn(
73-
`[DATE-diag] base _stringify called ` +
74-
`(${baseStringifyDiagCount}/${MAX_BASE_STRINGIFY_STACKS}) ` +
75-
`value=${valueStr} ` +
76-
`timezone=${options?.timezone ?? 'unset'} ` +
77-
`this.constructor.name=${dialectHint}\n` +
78-
stack,
79-
);
80-
}
8155
if (!moment.isMoment(date)) {
8256
date = this._applyTimezone(date, options);
8357
}

0 commit comments

Comments
 (0)