Page MenuHomePhabricator

Consider hiding (handled) warning about localStorage in mw.loader
Closed, ResolvedPublic

Description

See T148998#4233426.

When disabling cookies in Chrome and viewing a page, I do not get any uncaught exception or error in the console from mw.storage.

However, I do see the following:

Exception in store-localstorage-init:
> DOMException: Failed to read the 'localStorage' property from 'Window': Access is denied for this document.

This is not an error, and not from mw.storage. It is from mw.loader, and is logged as a warning (not an error) because it was caught and handled accordingly.

Perhaps we should suppress this particular warning, but in terms of behaviour and functionality, everything is working fine today.

When working with localStorage, the standard method is a try/catch. The code in question from mw.loader already does this:

mw.loader.store
try {
	raw = localStorage.getItem( mw.loader.store.getStoreKey() );
	// ..
) {
	mw.track( 'resourceloader.exception', { exception: e, source: 'store-localstorage-init' } );
}

We are currently intentionally logging it for debug purposes.

Shall we remove it?

Event Timeline

Krinkle created this task.May 26 2018, 10:14 AM
Restricted Application added a project: Performance-Team. · View Herald TranscriptMay 26 2018, 10:14 AM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Krinkle renamed this task from Consider hiding (handled) errors about localStorage in mw.loader to Consider hiding (handled) warning from localStorage in mw.loader.May 26 2018, 10:15 AM
Krinkle renamed this task from Consider hiding (handled) warning from localStorage in mw.loader to Consider hiding (handled) warning about localStorage in mw.loader.
Krinkle triaged this task as Low priority.Jun 26 2018, 5:05 PM
Krinkle moved this task from Inbox to Accepted Enhancement on the MediaWiki-ResourceLoader board.
Vvjjkkii renamed this task from Consider hiding (handled) warning about localStorage in mw.loader to o8baaaaaaa.Jul 1 2018, 1:08 AM
Vvjjkkii raised the priority of this task from Low to High.
Vvjjkkii updated the task description. (Show Details)
Vvjjkkii removed a subscriber: Aklapper.
Krinkle renamed this task from o8baaaaaaa to Consider hiding (handled) warning about localStorage in mw.loader.Jul 1 2018, 11:07 PM
Krinkle lowered the priority of this task from High to Low.
Krinkle updated the task description. (Show Details)
Krinkle added a subscriber: Aklapper.
Krinkle updated the task description. (Show Details)Mar 3 2019, 6:06 PM

Change 494049 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] resourceloader: Remove warning for handled localStorage error

https://gerrit.wikimedia.org/r/494049

Change 494049 merged by jenkins-bot:
[mediawiki/core@master] resourceloader: Remove warning for handled localStorage error

https://gerrit.wikimedia.org/r/494049

Krinkle closed this task as Resolved.Mar 4 2019, 4:44 PM
Krinkle removed a project: Patch-For-Review.