decentraland / catalyst-storage / 30203832702

push

github

fix!: correct read contracts, content sizes and S3 delete handling (#117)

* fix!: correct read contracts, content sizes and S3 delete handling

Reviewed the library for correctness and fixed what surfaced. The common
thread is storage faults being reported as "the content is not here",
which makes a broken node look like an empty one and stops it being
retried.

Bugs fixed:

- retrieve() reported the COMPRESSED size as a gzip item's contentSize,
  a field documented as the logical size and used by callers as
  `contentSize ?? size`. It now reads the trailer, matching fileInfo();
  S3 reports null, matching its own fileInfo().
- S3 delete() ignored the per-key Errors that DeleteObjects returns
  inside a 200 response, so it resolved while objects survived. It also
  broke on more than 1000 ids (S3's per-request cap, which the SDK does
  not chunk) and on an empty list.
- allFileIds() yielded bare basenames, so an id containing separators
  did not round-trip; the prefix filter matched filenames, including
  the .gzip extension, rather than ids.
- exist() used existPath (F_OK|R_OK) and so reported a present but
  unreadable file as absent, contradicting fileInfo() on the same id.
- The S3 backend turned every failure - 500, 503 SlowDown, expired
  credentials - into undefined/false.
- MIME detection swallowed every error, storing objects as
  application/octet-stream with nothing in the logs. This was actively
  failing in this repo's own test suite.
- A concurrent overwrite with smaller content made the gzip trailer read
  return fewer than 4 bytes, throwing ERR_BUFFER_OUT_OF_BOUNDS out of a
  healthy read; the path still existed, so it was not recognised as the
  benign race it is.
- An unrepairable quarantined id reported itself absent even though both
  representations were on disk and allFileIds() still listed it.
- Ids resolving to a directory rather than a file ('' and '.') were
  accepted; in flat mode they aliased onto the storage root.
- reco... (continued)

348 of 389 branches covered (89.46%)

Branch coverage included in aggregate %.

190 of 192 new or added lines in 11 files covered. (98.96%)

932 of 945 relevant lines covered (98.62%)

825.02 hits per line