1784446 - When using "storage.managed.get()" the promise is rejected if no manifest exists

Status: NEW

(WebExtensions :: Storage, defect, P5)

Description

[Manuel Reimer]

Maybe this is a "feature" but I can't test how this behaves on Windows as I have no Windows machines to test.

On Linux, as far as I know, the only way supported for "managed properties" is to use a manifest file. And if this file just does not exist, then the returned promise is rejected:

"Managed storage manifest not found"
https://searchfox.org/mozilla-central/source/toolkit/components/extensions/parent/ext-storage.js#296

Now I'm wondering what happens on Windows where more than this one way of setting the managed preferences is supported?
If just using the Add-on on Windows without doing any managed preferences setup does not cause the promise to be rejected, then a non-existing manifest file should also not cause the reject on Linux.

Current behaviour:
Using "storage.managed.get()" with no manifest file causes the returned promise to be rejected.

Expected behaviour:
No manifest file means no managed preferences. So I would expect the promise to be resolved with an empty object.

Could be an issue on Mac OS, too: https://github.com/sourcegraph/sourcegraph/issues/16339

Edit: The main reason why this is critical is that it seems like this bug does not exist on Windows. Probably because on Windows the "manifest file" is not the only target for getting managed properties. So if an Add-on, that uses the "storage.managed" feature, is only tested on Windows, then this Add-on will not work on anything besides Windows as the developer does not see the need for a "try/catch" block here.

Comment 1

[Rob Wu [:robwu]]

(In reply to Manuel Reimer from comment #0)

Maybe this is a "feature" but I can't test how this behaves on Windows as I have no Windows machines to test.

The behavior is identical on Windows.

On Chrome, the promise isn't rejected because default values can be derived from managed_schema, when nothing has been configured in the policy.
In Firefox, managed_schema is not supported (bug 1771731) and the current implementation of storage.managed offers access to the values configured by enterprise policies. Without support for managed_schema nor a specified policy, the implementation cannot return anything sensible.

Comment 2

[Manuel Reimer]

(In reply to Rob Wu [:robwu] from comment #1)

In Firefox, managed_schema is not supported (bug 1771731) and the current implementation of storage.managed offers access to the values configured by enterprise policies. Without support for managed_schema nor a specified policy, the implementation cannot return anything sensible.

Depends... In my opinion an Add-on does not even have to know if managed storage has not been configured. So in my opinion resolving the promise with an empty object seems to be absolutely "sensible".

Just like all the other "storage" APIs do. If I have never stored any setting to "storage.local", then "storage.local.get()" also returns an empty object.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:zombie, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 4

[Rob Wu [:robwu]]

The documentation for storageArea.get (for storage.managed.get) is inaccurate for Firefox;

https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/storage/StorageArea/get

If managed storage is not set, undefined will be returned.

This bug report shows that an error is reported instead.

I wonder whether we should update the documentation, or change the implementation.

Comment 5

[Rob Wu [:robwu]]

We should fix the documentation, to explain that in Firefox managed_storage is not supported (bug 1771731), and that therefore an error is thrown when storage.managed.get is called when there is no managed storage.

Developers who'd like to write code that is interoperable between browsers could do something like this:

let items = {
  someKey: "default value",
  anotherKey: "another default value",
};

try {
  items = await browser.storage.managed.get(items);
} catch (e) {
  console.error(`storage.managed not available: ${e}`);
  // managed storage not available, keep using the defaults.
}

// now use items here