CR-005/D6 fix - a bad CSV row rejects by line number instead of 500ing Postgres
Nick's real location list hit the production import and got 'Internal Server Error' with no line number - BL-027's class again, three days after the migration outage: Postgres enforces VARCHAR lengths and refuses control bytes, SQLite shrugs at both, and the importers were only ever rehearsed on SQLite. Reproduced both hazards locally (an over-long value and a NUL byte import cleanly on SQLite; either 500s Postgres wholesale). Both importers now validate per row, before any INSERT, so every dialect answers the same way - with the line number and a reason: - locations: control characters; names over 200; codes over 60; combined paths over 200 (checked where the path exists, with read-counts taken before the loop so a mid-loop rejection is not counted twice) - materials: control characters; description/unit/code over 300/20/80 And the client stops lying about it: wp-list-import.js read every response with r.json(), so a plain-text 500 threw mid-parse and surfaced as 'Could not reach the server' while the server was answering fine. One tolerant reader (text -> parse if it parses -> keep status) now serves import, add and patch; a real error reads 'Import refused - HTTP 500'. Pins: materials_check +2 (over-long and control-byte rows reject at line, 20/20), locations_check +1 (over-long name rejects at line, 59/59). Items: CR-005, D6, BL-027 (second instance of its class; the probe-side dialect guard it proposes is still open). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -2388,6 +2388,24 @@ def parse_location_rows(text: str) -> tuple[list[tuple[int, list[str]]], list[di
|
||||
rejected.append({"line": i, "text": line,
|
||||
"reason": "no letters or digits to make a code from"})
|
||||
continue
|
||||
# Postgres enforces VARCHAR lengths and refuses NUL/control bytes;
|
||||
# SQLite shrugs at both - which is how ONE bad CSV line 500'd the whole
|
||||
# production import (2026-08-23, BL-027's class again) instead of coming
|
||||
# back as a rejection with its line number. Validate per row, here,
|
||||
# so every dialect answers the same way: with a reason.
|
||||
if any(any(ord(ch) < 32 for ch in p) for p in parts):
|
||||
rejected.append({"line": i, "text": line[:120],
|
||||
"reason": "contains control characters — re-save the file as plain CSV (UTF-8)"})
|
||||
continue
|
||||
long_p = next((p for p in parts if len(p) > 200), None)
|
||||
if long_p is not None:
|
||||
rejected.append({"line": i, "text": line[:120],
|
||||
"reason": "a name is longer than 200 characters (%d)" % len(long_p)})
|
||||
continue
|
||||
if any(len(location_slug(p)) > 60 for p in parts):
|
||||
rejected.append({"line": i, "text": line[:120],
|
||||
"reason": "a code would be longer than 60 characters"})
|
||||
continue
|
||||
rows.append((i, parts))
|
||||
return rows, rejected
|
||||
|
||||
@@ -2454,6 +2472,7 @@ def import_locations(project_id: str, body: LocationImportIn,
|
||||
require_project_writable(db, user, project_id, "The location list cannot be changed")
|
||||
|
||||
rows, rejected = parse_location_rows(body.text)
|
||||
read_total = len(rows) + len(rejected)
|
||||
|
||||
existing = {n.path: n for n in db.scalars(
|
||||
select(models.LocationNode).where(models.LocationNode.project_id == project_id)
|
||||
@@ -2468,6 +2487,10 @@ def import_locations(project_id: str, body: LocationImportIn,
|
||||
for line_no, parts in rows:
|
||||
segs = [location_slug(p) for p in parts]
|
||||
full = "/".join(segs)
|
||||
if len(full) > 200:
|
||||
rejected.append({"line": line_no, "text": "/".join(parts)[:120],
|
||||
"reason": "the combined path is longer than 200 characters"})
|
||||
continue
|
||||
if full in seen_in_file:
|
||||
duplicates.append({"line": line_no, "path": full, "names": parts,
|
||||
"reason": "already on line %d of this import" % seen_in_file[full]})
|
||||
@@ -2510,7 +2533,7 @@ def import_locations(project_id: str, body: LocationImportIn,
|
||||
|
||||
result = {
|
||||
"project_id": project_id, "dry_run": bool(body.dry_run),
|
||||
"read": len(rows) + len(rejected),
|
||||
"read": read_total,
|
||||
"created": created, "duplicates": duplicates,
|
||||
"reactivated": reactivated, "rejected": rejected,
|
||||
}
|
||||
@@ -3007,6 +3030,18 @@ def parse_material_rows(text: str):
|
||||
rejected.append({"line": i, "text": raw.strip()[:120],
|
||||
"reason": "more than three columns - description, unit, code is the whole shape"})
|
||||
continue
|
||||
# Same guard as parse_location_rows: reject what Postgres would refuse
|
||||
# (VARCHAR limits, control bytes) with the line number, never a 500.
|
||||
if any(any(ord(ch) < 32 for ch in p) for p in parts):
|
||||
rejected.append({"line": i, "text": raw.strip()[:120],
|
||||
"reason": "contains control characters - re-save the file as plain CSV (UTF-8)"})
|
||||
continue
|
||||
caps = ((300, "description"), (20, "unit"), (80, "code"))
|
||||
long_col = next((("%s is longer than %d characters (%d)" % (label, cap, len(p)))
|
||||
for (cap, label), p in zip(caps, parts) if len(p) > cap), None)
|
||||
if long_col:
|
||||
rejected.append({"line": i, "text": raw.strip()[:120], "reason": long_col})
|
||||
continue
|
||||
rows.append((i, parts))
|
||||
return rows, rejected
|
||||
|
||||
|
||||
Reference in New Issue
Block a user