Skip to content

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

Merged
kptdobe merged 1 commit into
mainfrom
verlog
May 22, 2026
Merged

fix(version): log diagnostics on POST /versionsource silent 500#284
kptdobe merged 1 commit into
mainfrom
verlog

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

Replaces #283 (which referenced an internal tracker id in its branch name; closed without merge).

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>
@kptdobe
kptdobe merged commit f06504b into main May 22, 2026
11 checks passed
@kptdobe
kptdobe deleted the verlog branch May 22, 2026 08:53
adobe-bot pushed a commit that referenced this pull request May 22, 2026
## [1.9.2](v1.9.1...v1.9.2) (2026-05-22)

### Bug Fixes

* **version:** log diagnostics on POST /versionsource silent 500 ([#284](#284)) ([f06504b](f06504b))
@adobe-bot

Copy link
Copy Markdown
Collaborator

🎉 This PR is included in version 1.9.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

kptdobe added a commit that referenced this pull request May 29, 2026
…e gate

Pivot from the gate-widening approach (createVersion || label != null) to
healing the underlying metadata on the labelled-version path.

postObjectVersionWithLabel now derives a versionable mime from daCtx.ext
when the stored ContentType is missing or application/octet-stream:

  html  -> text/html
  json  -> application/json

The inferred type is passed via update.type. shouldCreateVersion sees the
healed type, the version snapshot stores ContentType: text/html (or json),
and the main object's PUT overwrites the stale ContentType in S3 metadata
so the file is self-healed for all future requests.

Binary files (jpg/pdf/etc.) still cannot be labelled-versioned, matching
the project's "binaries do not version" semantics. The diagnostic log from
#284 is retained and extended with inferredType + ext, so the unhealed
path is observable.

Tests updated:
- new: legacy octet-stream HTML labelled version heals snapshot + main
- new: labelled version on non-versionable ext still 500s with diagnostic
- companion: plain PUT auto-version gate intact (no leak from labelled path)

Co-Authored-By: Paperclip <noreply@paperclip.ing>
kptdobe added a commit that referenced this pull request May 29, 2026
…led version (#288)

* fix(version): create labelled versions for non-html/json content types

POST /versionsource returned a silent 500 for legacy imports whose
source object was stored with ContentType: application/octet-stream.
shouldCreateVersion gates only text/html and application/json, so even
when an explicit label was supplied the version write was skipped and
postObjectVersionWithLabel returned { error: 'Version was not created' }.

The diagnostic added in #284 confirmed 9 occurrences/24h with an
identical fingerprint (octet-stream, hadLabel, currentStatus=200).

When the caller passes an explicit label, treat the version as
requested-by-name and create it regardless of contentType. Auto-version
on plain PUT still gates to html/json, so storage cost is bounded to
the labelled call rate.

Refs: COR-55, COR-46

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* test(version): drop ticket-id references from test names and comments

Project convention: ticket IDs belong in commit messages and PR
descriptions, not source code (they rot as tickets are renumbered or
deleted).

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(version): infer ContentType from extension instead of widening the gate

Pivot from the gate-widening approach (createVersion || label != null) to
healing the underlying metadata on the labelled-version path.

postObjectVersionWithLabel now derives a versionable mime from daCtx.ext
when the stored ContentType is missing or application/octet-stream:

  html  -> text/html
  json  -> application/json

The inferred type is passed via update.type. shouldCreateVersion sees the
healed type, the version snapshot stores ContentType: text/html (or json),
and the main object's PUT overwrites the stale ContentType in S3 metadata
so the file is self-healed for all future requests.

Binary files (jpg/pdf/etc.) still cannot be labelled-versioned, matching
the project's "binaries do not version" semantics. The diagnostic log from
#284 is retained and extended with inferredType + ext, so the unhealed
path is observable.

Tests updated:
- new: legacy octet-stream HTML labelled version heals snapshot + main
- new: labelled version on non-versionable ext still 500s with diagnostic
- companion: plain PUT auto-version gate intact (no leak from labelled path)

Co-Authored-By: Paperclip <noreply@paperclip.ing>

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>
adobe-bot pushed a commit that referenced this pull request May 29, 2026
## [1.9.3](v1.9.2...v1.9.3) (2026-05-29)

### Bug Fixes

* **version:** heal ContentType for legacy octet-stream files on labelled version ([#288](#288)) ([bf86d40](bf86d40)), closes [#284](#284) [#284](#284)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants