From 610bcf08cdaec76b1a980f203c3b796bfcb20053 Mon Sep 17 00:00:00 2001 From: juliet Date: Tue, 14 Jul 2026 22:09:40 -0400 Subject: [PATCH] Fix for Manual Review Tool: If createdAt datetime is in bad format, store it as null in decision log (#913) * fix(mrt): store null for unparseable item createdAt in decision log Submitting a decision failed with a 500 ("Job submission failed. Please try again.") for any job whose item carries a truthy but unparseable createdAt value. `#logDecision` passed `new Date(itemCreatedAtField)` straight into the `item_created_at` timestamptz column; an unparseable value yields an Invalid Date, which the pg driver serializes to a NaN string that Postgres rejects (22007), failing the whole decision insert. The decision is never recorded and the job is never removed, so the task is stuck in the queue. Normal item submissions can't reach this state because the DATETIME field handler validates dates at intake. A bad value only arrives via a path that skips that validation (e.g. system-generated reports, or a createdAt role mapped to a non-DATETIME field). Normalize at the write boundary: parse the value and store null when it is not a valid date. The column is already nullable, so the decision records and the task clears. Adds a unit regression test for the normalization helper. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01CEU6YHFiBYfTqjgM5Vyt9n * docs(changelog): note the unparseable createdAt decision fix Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01CEU6YHFiBYfTqjgM5Vyt9n * fix(mrt): preserve epoch 0 in parseItemCreatedAt Address review: `!value` treated a numeric 0 (a valid 1970-01-01 epoch) as empty. Guard only null/undefined/empty-string instead, and cover epoch 0 in the test. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01CEU6YHFiBYfTqjgM5Vyt9n * fix(mrt): record unparseable item createdAt values Address review: surface invalid createdAt values instead of silently nulling them. When a present createdAt can't be parsed, emit a tracer span (job id, org id, the raw value) so the bad data is diagnosable and can be backfilled. The decision still saves with a null item_created_at. new Date() returns an Invalid Date rather than throwing, so this detects the invalid parse rather than wrapping in try/catch. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01CEU6YHFiBYfTqjgM5Vyt9n --------- Co-authored-by: Claude Opus 4.8 --- CHANGELOG.md | 4 ++ .../modules/JobDecisioning.test.ts | 40 ++++++++++++++ .../modules/JobDecisioning.ts | 53 +++++++++++++++++-- 3 files changed, 94 insertions(+), 3 deletions(-) create mode 100644 server/services/manualReviewToolService/modules/JobDecisioning.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 69f1c51..7fa07e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ **Full Changelog**: https://github.com/roostorg/coop/compare/1.0.2...main +## Review Console + +- Fixed a "Job submission failed" error that prevented reviewers from clearing jobs whose item had an unparseable `Created At` value; the decision now records instead of failing (#913) + # Coop 1.0.2 This release addresses reported security advisories, improves NCMEC CyberTipline reporting, and includes front-end quality-of-life improvements. diff --git a/server/services/manualReviewToolService/modules/JobDecisioning.test.ts b/server/services/manualReviewToolService/modules/JobDecisioning.test.ts new file mode 100644 index 0000000..2efbdf9 --- /dev/null +++ b/server/services/manualReviewToolService/modules/JobDecisioning.test.ts @@ -0,0 +1,40 @@ +import { parseItemCreatedAt } from './JobDecisioning.js'; + +describe('parseItemCreatedAt', () => { + test('parses a valid ISO string', () => { + expect(parseItemCreatedAt('2026-01-01T00:00:00.000Z')).toEqual( + new Date('2026-01-01T00:00:00.000Z'), + ); + }); + + test('parses an epoch-millis number', () => { + expect(parseItemCreatedAt(1735689600000)).toEqual(new Date(1735689600000)); + }); + + test('treats epoch 0 as a valid timestamp, not empty', () => { + expect(parseItemCreatedAt(0)).toEqual(new Date(0)); + }); + + test('passes a Date through', () => { + const d = new Date('2026-01-01T00:00:00.000Z'); + expect(parseItemCreatedAt(d)).toEqual(d); + }); + + test.each([null, undefined, ''])( + 'returns null for empty value %p', + (value) => { + expect(parseItemCreatedAt(value)).toBeNull(); + }, + ); + + // Regression: a truthy-but-unparseable createdAt (seen on reports from + // automated sources) produced an Invalid Date, which throws on pg + // serialization and failed the entire decision insert, surfacing as + // "Job submission failed" in the reviewer UI. + test.each([' ', 'not-a-date', 'garbage', '2026-99-99T99:99:99Z'])( + 'returns null for unparseable value %p instead of an Invalid Date', + (value) => { + expect(parseItemCreatedAt(value)).toBeNull(); + }, + ); +}); diff --git a/server/services/manualReviewToolService/modules/JobDecisioning.ts b/server/services/manualReviewToolService/modules/JobDecisioning.ts index 1be10fe..1081ce7 100644 --- a/server/services/manualReviewToolService/modules/JobDecisioning.ts +++ b/server/services/manualReviewToolService/modules/JobDecisioning.ts @@ -5,12 +5,14 @@ import { type JsonObject } from 'type-fest'; import { type Dependencies } from '../../../iocContainer/index.js'; import { filterNullOrUndefined } from '../../../utils/collections.js'; +import { jsonStringify } from '../../../utils/encoding.js'; import { CoopError, ErrorType, type ErrorInstanceData, } from '../../../utils/errors.js'; import { assertUnreachable } from '../../../utils/misc.js'; +import { isValidDate } from '../../../utils/time.js'; import { isNonEmptyString } from '../../../utils/typescript-types.js'; import { getFieldValueForRole } from '../../itemProcessingService/index.js'; import { type NCMECMediaReport } from '../../ncmecService/ncmecReporting.js'; @@ -75,6 +77,24 @@ type MRTJobAutoCloseReason = */ export const AUTOMATED_DECISION_REVIEWER_ID = ''; +/** + * Normalizes an item's createdAt field value into a Date for the + * `item_created_at` column. A truthy-but-unparseable value (whitespace, or a + * non-ISO format some report sources emit) yields an Invalid Date, which throws + * on pg serialization and fails the entire decision insert. Return null in that + * case so the nullable column absorbs it and the decision still records. + */ +export function parseItemCreatedAt( + value: string | number | Date | null | undefined, +): Date | null { + // Guard only null/undefined/empty-string; a numeric 0 is a valid epoch. + if (value == null || value === '') { + return null; + } + const parsed = new Date(value); + return isValidDate(parsed) ? parsed : null; +} + export type ManualReviewDecisionComponent = | { type: 'IGNORE' } | { @@ -627,9 +647,36 @@ export default class JobDecisioning { job.payload.item.data, ) : null; - const itemCreatedAt = itemCreatedAtField - ? new Date(itemCreatedAtField) - : null; + const itemCreatedAt = parseItemCreatedAt(itemCreatedAtField); + + // Record when a present createdAt couldn't be parsed. We store null so the + // decision still saves, but surface the bad value so it's diagnosable and + // can be backfilled rather than silently dropped. + if ( + itemCreatedAt === null && + itemCreatedAtField != null && + itemCreatedAtField !== '' + ) { + this.tracer.addSpan( + { + resource: 'mrtService', + operation: 'logDecision.invalidItemCreatedAt', + }, + (span) => { + span.setAttribute('job.id', job.id); + span.setAttribute('org.id', orgId); + this.tracer.logSpanFailed( + span, + new Error( + `Unparseable item createdAt for job ${job.id}: ${jsonStringify( + itemCreatedAtField, + )}. Storing null.`, + ), + ); + return null; + }, + ); + } return this.pgQuery .insertInto('manual_review_tool.manual_review_decisions') -- 2.51.2