1704432 - [QM_TRY] Failures in dom/quota/ActorsParent.cpp:LoadLocalStorageArchiveVersion

Status: RESOLVED WORKSFORME

(Core :: Storage: Quota Manager, defect, P3)

Description

[Jens Stutte [:jstutte]]

Clients	Sessions	Hits	Anchor	Stack
2	2	2	dom/quota/ActorsParent.cpp:LoadLocalStorageArchiveVersion	dom/quota/ActorsParent.cpp#667:None <- dom/quota/ActorsParent.cpp#5875:NS_ERROR_FILE_CORRUPTED

Comment 1

[Jens Stutte [:jstutte]]

From what I read in the code I would have expected to see some NS_ERROR_* result already here coming from the previous call?

  QM_TRY_INSPECT(const auto& stmt,
                 CreateAndExecuteSingleStepStatement<
                     SingleStepResult::ReturnNullIfNoResult>(
                     aConnection, "SELECT version FROM database"_ns));

  QM_TRY(OkIf(stmt), Err(NS_ERROR_FILE_CORRUPTED));   <---------- We fail here but without error?

Comment 2

[Jan Varga [:janv]]

Jens, OkIf function returns Result<Ok, NotOk>. Ok and NotOk are just empty structs. There's no error code. The None string may be confusing, it's not that there's no error.

Comment 3

[Jens Stutte [:jstutte]]

IIUC, the OkIf condition is the underlying error here, that is stmt == nullptr. So actually we have an arbitrary error during QM_TRY_INSPECT that makes stmt to be returned as nullptr (due to SingleStepResult::ReturnNullIfNoResult), which we then intercept and interpret as NS_ERROR_FILE_CORRUPTED, ignoring what really happened to our SELECT statement.

Comment 4

[Jens Stutte [:jstutte]]

Hi Jan, this seems to have ceased since two months now. Did we do something to the reporting here?

Comment 5

[Jan Varga [:janv]]

(In reply to Jens Stutte [:jstutte] from comment #4)

Hi Jan, this seems to have ceased since two months now. Did we do something to the reporting here?

This probably disappeared because we converted ordinary orElse to QM_WARN_OR_ELSE. The corruption is handled by creating an empty archive, but before the special macro, the corruption was reported as an error. It's now moved to warnings in QM_TRY reports because QM_WARN_OR_ELSE adds a special warning event to the telemetry.

See:
https://hg.mozilla.org/releases/mozilla-beta/annotate/e4ba3bcb9d881637a2c30daf7fa9f544087ba181/dom/quota/ActorsParent.cpp#l6034 https://hg.mozilla.org/mozilla-central/file/bb08fc2c750e013498236a40c2f8eed5c99538d5/dom/quota/ActorsParent.cpp#l6019