lib: throw from localStorage getter on missing storage path

[Renegade334]

lib: throw from localStorage getter on missing storage path

The behavior of the global localStorage variable has changed, in the specific case when webstorage is enabled but the --localstorage-file option is not provided. In Node.js v25.0.0 and v25.1.0, this would cause localStorage to be initialized as an empty object. In v25.2.0, an exception is now thrown when the localStorage variable is accessed, which is more consistent with the web standard.

Fixes #60303.

The Storage specification states that the global localStorage getter should throw an exception if the storage environment cannot be obtained. Accordingly, a common way of testing for local storage capability in userland is along these lines:

function localStorageIsAvailable() {
  try {
    return typeof localStorage !== 'undefined';
  } catch {
    return false;
  }
}

This tests both for the presence of the global and whether the getter succeeds, which also covers scenarios where the storage API is not exposed at all.

Node.js's flagged implementation of localStorage also used to follow this behaviour in v24 and earlier. If local storage could not be initialised due to the absence of the --localstorage-file command-line argument, the global getter would throw at the point of the variable being accessed. The above test therefore also worked well in v24: it would identify when the variable didn't exist, and would also detect when an accessor exception was thrown.

When webstorage was enabled by default for v25 (#57666), this behaviour was changed: in the absence of --localstorage-file, the global getter no longer throws, but instead returns an empty Proxy which returns undefined on all property access after emitting a warning. The motivation behind this change of approach was to avoid the test suite throwing during its global tests. However, a throw-on-access global getter already exists (crypto), and therefore there already exist mechanisms for getting around this.

The behaviour as it stands is problematic. Exposing localStorage as an empty object with no observable properties is an unexpected pattern, is inconsistent with the web, is non-compliant with the spec, and there is no longer a trivial and web-consistent way to observe localStorage to work out whether or not it's actually available.

I am therefore proposing, as an out-of-cycle breaking change, to revert to web-compatible behaviour and throw from the global getter if localStorage cannot be initialised due to the missing command-line argument. Edit: just realised that webstorage wasn't stabilised, so this wouldn't need TSC approval to land.

cc: @nodejs/web-standards

[KhafraDev]

Shouldn't this throw a SecurityError DOMException rather than a plain error?

Throws a "SecurityError" DOMException if the Document's origin is an opaque origin or if the request violates a policy decision (e.g., if the user agent is configured to not allow the page to persist data).

https://html.spec.whatwg.org/multipage/webstorage.html#dom-localstorage-dev

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.56%. Comparing base (bdf03bf) to head (600eae6).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #60351      +/-   ##
==========================================
+ Coverage   88.05%   88.56%   +0.50%     
==========================================
  Files         704      704              
  Lines      207857   207840      -17     
  Branches    39964    40054      +90     
==========================================
+ Hits       183030   184073    +1043     
+ Misses      16806    15798    -1008     
+ Partials     8021     7969      -52     

Files with missing lines	Coverage Δ
lib/internal/webstorage.js	100.00% <100.00%> (+8.69%)	⬆️

... and 107 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Renegade334]

Shouldn't this throw a SecurityError DOMException rather than a plain error?

Throws a "SecurityError" DOMException if the Document's origin is an opaque origin or if the request violates a policy decision (e.g., if the user agent is configured to not allow the page to persist data).

https://html.spec.whatwg.org/multipage/webstorage.html#dom-localstorage-dev

I did go with this originally, but a) the security semantics don't really apply here (there's no origin check, for instance); b) throwing a native Error for an invalid execution environment is consistent with Node's other web implementations, eg..the global crypto getter.

[KhafraDev]

a) the security semantics don't really apply here (there's no origin check, for instance);

The spec reads as "throw a SecurityError if 'security semantics' or 'not configured to store data'" which makes it seem to me like we should still throw a SecurityError DOMException.

the global crypto getter.

I think webstorage is special here, can crypto throw in browsers/the spec?

[Renegade334]

I'll link the original DOMException-throwing commit (5e04491) for review alongside. If there's consensus to use this approach, then it's no problem to change it.

[JakobJingleheimer]

LGTM 🙌

[JakobJingleheimer]

Nit: matching against the whole error message is brittle. Better to match against only ERR_WEBSTORAGE_MISSING_STORAGE_PATH.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/69922/

[gurgunday]

Lgtm

[mcollina]

lgtm

[github-actions]

The notable-change PRs with changes that should be highlighted in changelogs. label has been added by @RafaelGSS.

Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section.

[KhafraDev]

We should be throwing a DOMException.

[Renegade334]

Modified as per the blocking review.

Note that this does diverge the behaviours of --localstorage-file being absent (DOMException) and being invalid/inaccessible (Error) – the latter is thrown from the failed call to sqlite3_open.

[mcollina]

lgtm

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/69965/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/69970/

[nodejs-github-bot]

Landed in 2fb3c7e

[tophf]

FWIW the spec's throwing on getter behavior breaks {...global} used by some packages (html-webpack-plugin), so I wonder if the spec should be changed to match Node's previous behavior?

[Renegade334]

the spec's throwing on getter behavior breaks {...global} used by some packages

This isn't novel; the same occurs if eg. Node.js is built without crypto support. Global getters throwing errors is acceptable behaviour in the web spec, and implementations need to account for this.

[tophf]

I see, thanks.

FWIW, const shallowCopy = Object.defineProperties({}, Object.getOwnPropertyDescriptors(global)) is a spec-compliant no-loop alternative that also copies the non-enumerable props, making it even better than ... for the case of global specifically.

[aconti-ns1]

FWIW we resolved the issue by adding NODE_OPTIONS='--localstorage-file=./localstorage' to our build environment. (via @apravehum)

[slorber]

I agree that the current Node change is spec-complient: https://html.spec.whatwg.org/multipage/webstorage.html#the-localstorage-attribute

Throws a "SecurityError" DOMException if the Document's origin is an opaque origin or if the request violates a policy decision (e.g., if the user agent is configured to not allow the page to persist data).

However, in practice, this is quite annoying in terms of DX, and requires us to gate our code everywhere to protect against localStorage throwing.

Sure, we can implement such a function:

function localStorageIsAvailable() {
  try {
    return typeof localStorage !== 'undefined';
  } catch {
    return false;
  }
}

But this is going to lead to unperformant code, creating expensive stacktraces for regular program control flow.

It's unlikely that this code will only run once per node app and be memoized, considering all third-party apps needing localStorage access will have to write a similar function.

We do have such code on Docusaurus, which was initially introduced for browser cases (in particular, when running within an iframes with certain browser settings):

  try {
    return window[storageType];
  } catch (err) {
    logOnceBrowserStorageNotAvailableWarning(err as Error);
    return null;
  }

---

It's already annoying that browsers can throw on getter access, and IMHO that choice is questionable and probably historical.

It will probably be hard to find out why they made that choice decades ago, but here's a guess from someone working on web platform APIs, and suggesting this could be something to discuss in Winter TC (TC55)

https://bsky.app/profile/lukewarlow.dev/post/3m5ldcvizw22c

For what it's worth, Deno doesn't throw:

---

By the way, it feels inconsistent with the spec to only throw for localStorage, and not for sessionStorage: https://html.spec.whatwg.org/multipage/webstorage.html#the-sessionstorage-attribute