Skip to content

fix(version): log diagnostics on POST /versionsource silent 500#283

Closed
kptdobe wants to merge 1 commit into
mainfrom
cor46log
Closed

fix(version): log diagnostics on POST /versionsource silent 500#283
kptdobe wants to merge 1 commit into
mainfrom
cor46log

Conversation

@kptdobe

@kptdobe kptdobe commented May 22, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds a single console.error in postObjectVersionWithLabel before the existing silent return { status: 500, error: "Version was not created" } branch, capturing { contentType, hadLabel, currentStatus } so the failure shows up in Cloudflare Logs instead of arriving empty.
  • No behavior change otherwise -- same 500 status, same error string returned to the caller.

Why

We saw a cluster of POST /versionsource returning 500 with empty Logs[], empty Exceptions[], and Outcome == ok, concentrated on legacy-imported HTML pages. After #271 closed the 412-race silent-500, the only remaining path to that branch is putObjectWithVersion returning { status: 200, versionCreated: false }, which requires shouldCreateVersion(contentType) to be false. For these legacy-imported HTML pages the S3 object metadata is missing ContentType, so current.contentType is undefined -> createVersion=false -> no version object is written -> silent 500.

Without instrumentation, the next log review cannot tell whether the cluster is contentType-missing, label-related, or a regression. This adds the cheapest possible diagnostic so the theory can be confirmed (or refuted) from logs alone -- no code archaeology, no client repro needed.

Approach

  • Failing test first: asserts postObjectVersionWithLabel emits a structured console.error with contentType, hadLabel, currentStatus when the source object has no contentType. Confirmed it failed on main before the implementation change.
  • Implementation: destructure status: currentStatus from the outer getObject call and emit one console.error inside the !resp.versionCreated branch.
  • Out of scope, deliberately deferred: ext-based fallback in shouldCreateVersion and/or repairing the missing ContentType metadata. Get diagnostic data to confirm root cause before changing the gate.

Test plan

  • npm run lint clean on touched files
  • npm test -- 392 passing including the new regression; 100% line coverage on src/storage/version/put.js
  • Post-deploy: re-run the diagnostic log query 24h later -- the new Failed to version (no version created) row should appear with the dominant URL set, confirming the contentType-missing theory.

Refs

@codecov

codecov Bot commented May 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

When postObjectVersionWithLabel reaches the `return 500, 'Version was
not created'` branch, no console.error is emitted, so the failure
arrives in Cloudflare Logs with empty Logs[] / Exceptions[]. Forensics
on a recent cluster of these failures on legacy HTML imports traced
them to source objects missing ContentType metadata:
shouldCreateVersion(undefined) is false, putObjectWithVersion returns
{ status: 200, versionCreated: false }, and the silent 500 falls out.

Adds a single, scoped diagnostic log capturing contentType, hadLabel,
and the source-object status so the next log review can confirm the
contentType-missing theory without further code archaeology.

Test asserts the silent-500 path now emits a structured log row with
the expected fields when the source contentType is undefined.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant