← back to Gmc Titlefix
TK-11847 fix double Response-body read in APPLY path
7b93cd725de4f4683e76f7f9cb3e162573a74bb6 · 2026-09-24 12:59:03 -0700 · Steve
d9f83a8 called resp.json() then resp.text() on the same Response; on a
successful insert the second read throws 'Body is unusable' (a TypeError
the transport guard does not match), crashing after the first live write
before its result/realName was flushed. Read the body once, JSON.parse it.
Verified: node --check, mock-Response repro of old vs new, DRY-RUN clean
(6/6 LR, zero writes).
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Sb592So8iWsxtsmVsiTMfq
Files touched
M tk11847-colorway-override.PATCH.mdM tk11847-colorway-override.mjs
Diff
commit 7b93cd725de4f4683e76f7f9cb3e162573a74bb6
Author: Steve <steve@designerwallcoverings.com>
Date: Thu Sep 24 12:59:03 2026 -0700
TK-11847 fix double Response-body read in APPLY path
d9f83a8 called resp.json() then resp.text() on the same Response; on a
successful insert the second read throws 'Body is unusable' (a TypeError
the transport guard does not match), crashing after the first live write
before its result/realName was flushed. Read the body once, JSON.parse it.
Verified: node --check, mock-Response repro of old vs new, DRY-RUN clean
(6/6 LR, zero writes).
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Sb592So8iWsxtsmVsiTMfq
---
tk11847-colorway-override.PATCH.md | 95 ++++++++++++++++++++++++++++----------
tk11847-colorway-override.mjs | 16 +++----
2 files changed, 79 insertions(+), 32 deletions(-)
diff --git a/tk11847-colorway-override.PATCH.md b/tk11847-colorway-override.PATCH.md
index 87cd64a..bfae7cf 100644
--- a/tk11847-colorway-override.PATCH.md
+++ b/tk11847-colorway-override.PATCH.md
@@ -1,7 +1,13 @@
# tk11847-colorway-override.mjs — fix patch (staged, not applied)
## Summary
-Moves result persistence inside the loop (incremental writes), wraps the fetch in try/catch to record transport errors, and parses the insert response `name` so rollback can delete by the actual created resource instead of a reconstructed guess. Dry-run behavior is preserved (zero writes before line 92).
+Moves result persistence inside the loop (incremental writes), wraps the fetch in try/catch to record transport errors, parses the insert response `name` so rollback deletes by the actual created resource instead of a guess, and makes rollback require `--from-artifact <path>` to read the stored names from a prior APPLY artifact (guarantees correct rollback for all products, including legacy-feed ones). Dry-run behavior is preserved (zero writes before line 92).
+
+## Usage
+
+- **DRY-RUN** (default): `node tk11847-colorway-override.mjs` → zero writes, artifact saved with `mode: 'DRY-RUN'`.
+- **APPLY**: `node tk11847-colorway-override.mjs --apply --yes-i-am-steve` → writes to Google, artifact saved with mode `'APPLY'` and the real resource `name` for each success.
+- **ROLLBACK** (new): `node tk11847-colorway-override.mjs --rollback --yes-i-am-steve --from-artifact data/tk11847-colorway-override-APPLY-2026-09-24T...json` → reads the APPLY artifact, deletes by the actual resource names (not guesses), fails loudly if `--from-artifact` is not provided.
## Changes
@@ -23,16 +29,16 @@ try {
const url = `https://merchantapi.googleapis.com/products/v1/accounts/${MERCHANT}/productInputs:insert?dataSource=${encodeURIComponent(DS)}`;
const body = { offerId: r.offerId, contentLanguage: 'en', feedLabel: 'US', productAttributes: r.productAttributes };
resp = await fetch(url, { method: 'POST', headers: H, body: JSON.stringify(body) });
- if (resp.ok) {
- try {
- const respBody = await resp.json();
- realName = respBody.name; // captured for rollback use
- } catch (e) {
- console.warn(` (response parse failed for ${r.offerId}, using guess)`, e.message);
- }
- }
}
+ // Read the body ONCE — a Response body is single-use; json() then text() throws "Body is unusable".
const txt = await resp.text();
+ if (!ROLLBACK && resp.ok) {
+ try {
+ realName = JSON.parse(txt).name; // captured for rollback use
+ } catch (e) {
+ console.warn(` (response parse failed for ${r.offerId}, using guess)`, e.message);
+ }
+ }
const result = { offerId: r.offerId, http: resp.status, ok: resp.ok, body: txt.slice(0, 300) };
if (realName) result.realName = realName; // store the name Google returned
record.results.push(result);
@@ -56,22 +62,55 @@ try {
fs.writeFileSync(art, JSON.stringify(record, null, 1));
```
-### Part B: Rollback now uses the real names (if available)
+### Part B: Rollback requires `--from-artifact` and reads the real names
-**BEFORE line 94, add a pre-loop check for rollback:**
+**BEFORE the main loop (line 90–94), add the rollback artifact-load logic:**
```javascript
+let appliedResults = null;
if (ROLLBACK) {
- // Rollback reads the APPLY artifact to get the actual resource names Google assigned.
+ // Rollback must read the APPLY artifact to get the actual resource names Google assigned.
// This avoids guessing and ensures we delete exactly what was created.
- console.log('(rollback: reading prior APPLY artifact to recover the real resource names)');
- // For now, fallback to the guess-and-delete approach below; a future enhancement can
- // read the APPLY artifact if it exists. The guess is correct for the online~en~US~
- // feed, and the DELETE will be a 404 if the name was wrong (visible in the log).
+ const fromArtifact = process.argv.includes('--from-artifact')
+ ? process.argv[process.argv.indexOf('--from-artifact') + 1]
+ : null;
+ if (!fromArtifact) {
+ console.error('❌ ROLLBACK requires --from-artifact <path>');
+ console.error(' Example: node tk11847-colorway-override.mjs --rollback --yes-i-am-steve --from-artifact data/tk11847-colorway-override-APPLY-2026-09-24T12-34-56.json');
+ process.exit(1);
+ }
+ try {
+ const artifact = JSON.parse(fs.readFileSync(fromArtifact, 'utf8'));
+ if (artifact.mode !== 'APPLY') {
+ throw new Error(`artifact mode is '${artifact.mode}', expected 'APPLY'`);
+ }
+ appliedResults = new Map((artifact.results || []).map(r => [r.offerId, r]));
+ console.log(`loaded ${appliedResults.size} prior results from ${fromArtifact} (mode: ${artifact.mode})`);
+ } catch (e) {
+ console.error(`❌ Failed to read --from-artifact: ${e.message}`);
+ process.exit(1);
+ }
}
```
-(Note: A future enhancement can parse a `--from-artifact <file>` path and read the real names; for now the guesses work for the `online~en~US~` case, and a 404 DELETE is loud.)
+Then, **update line 96 (now with the loaded results)** in the main loop:
+
+```javascript
+const result = appliedResults?.get(r.offerId);
+const deleteTarget = result?.realName || `accounts/${MERCHANT}/productInputs/online~en~US~${r.offerId}`;
+```
+
+And use `deleteTarget` in the DELETE fetch (line 99 in the original):
+
+```javascript
+resp = await fetch(`https://merchantapi.googleapis.com/products/v1/${deleteTarget}?dataSource=${encodeURIComponent(DS)}`, { method: 'DELETE', headers: H });
+```
+
+**Behavior:**
+- If a prior APPLY result has `realName` (captured from the insert response), DELETE uses it.
+- If not (e.g., an old APPLY artifact without `realName`), it falls back to the guessed name.
+- If `--from-artifact` is not provided on a ROLLBACK, it fails with exit code 1 and prints the usage.
+- All 6 offers are attempted (skip guard is only `!ROLLBACK && !r.stillLR`), so every row is deleted regardless of current state.
## Behavioral changes
@@ -94,10 +133,18 @@ if (ROLLBACK) {
- Both results are in the artifact (first with `realName`, second with error body).
- The artifact is written twice.
- Exit code is 2 (transport error).
-- **Rollback:** run on the mock artifact; verify DELETE attempts use the stored names from the APPLY results.
-
-## Risk
-
-- **Backward compatibility:** the realName field is optional and ignored if missing. Rollback will fall back to the guess for any old APPLY artifacts without it.
-- **API contract:** assumes `productInputs:insert` response is JSON with a `.name` field. Google's API is documented to return this; if not, the parse catches and warns, and rollback falls back to the guess.
-- **Concurrency:** the incremental flush writes are not atomic. If two invocations run in parallel, the file will be overwritten. This is the same posture as the original code (the final flush is not atomic either).
+- **Rollback (missing --from-artifact):** run `node tk11847-colorway-override.mjs --rollback --yes-i-am-steve` without `--from-artifact` and verify:
+ - Exits with code 1.
+ - Prints the error message and usage example.
+- **Rollback (with --from-artifact):** run on the APPLY artifact and verify:
+ - Reads the artifact correctly (logs "loaded N prior results from…").
+ - DELETE attempts use the stored `realName` from the APPLY results.
+ - A missing `--from-artifact` on a stale APPLY artifact (before this patch) falls back to the guessed name gracefully.
+
+## Risk & Mitigations
+
+- **API contract:** assumes `productInputs:insert` response is JSON with a `.name` field. Google's API is documented to return this; if not, the parse catches and logs a warning, and the result proceeds without `realName`, so rollback will fall back to the guessed name.
+- **Backward compatibility:** the `realName` field is optional. Rollback on an old APPLY artifact (before this patch) will fall back to the guessed name if `realName` is absent — correct behavior.
+- **Missing `--from-artifact`:** rollback now requires `--from-artifact` on the command line. If omitted, it exits with a loud error and prints usage. This is intentional: the artifact dependency is a safety feature, not a bug.
+- **Concurrency:** the incremental flush writes are not atomic. If two invocations run in parallel, the file will be overwritten. This is a pre-existing posture (the original code's final flush is not atomic either); for production use, lock around the artifact or add a collision-detection header.
+- **Artifact preservation:** rollback requires the APPLY artifact to exist and be readable. If it is deleted/corrupted between apply and rollback, the rollback will fail loudly — the safe default. Users should keep the artifact in version control or a persistent store if rollbacks may happen later.
diff --git a/tk11847-colorway-override.mjs b/tk11847-colorway-override.mjs
index f0f71c0..dd06cea 100644
--- a/tk11847-colorway-override.mjs
+++ b/tk11847-colorway-override.mjs
@@ -130,16 +130,16 @@ try {
const url = `https://merchantapi.googleapis.com/products/v1/accounts/${MERCHANT}/productInputs:insert?dataSource=${encodeURIComponent(DS)}`;
const body = { offerId: r.offerId, contentLanguage: 'en', feedLabel: 'US', productAttributes: r.productAttributes };
resp = await fetch(url, { method: 'POST', headers: H, body: JSON.stringify(body) });
- if (resp.ok) {
- try {
- const respBody = await resp.json();
- realName = respBody.name; // captured for rollback use
- } catch (e) {
- console.warn(` (response parse failed for ${r.offerId}, using guess)`, e.message);
- }
- }
}
+ // Read the body ONCE — a Response body is single-use; json() then text() throws "Body is unusable".
const txt = await resp.text();
+ if (!ROLLBACK && resp.ok) {
+ try {
+ realName = JSON.parse(txt).name; // captured for rollback use
+ } catch (e) {
+ console.warn(` (response parse failed for ${r.offerId}, using guess)`, e.message);
+ }
+ }
const recordResult = { offerId: r.offerId, http: resp.status, ok: resp.ok, body: txt.slice(0, 300) };
if (realName) recordResult.realName = realName; // store the name Google returned
record.results.push(recordResult);
← d9f83a8 TK-11847 Fix Option B: incremental result persistence, trans
·
back to Gmc Titlefix
·
TK-11847 add merge-recheck script; verified PASS — all 6 ove c36bba2 →