Luckily, I've had a reprieve from Ancient Browser Testing (let them use Notebook Classic!) for a while, but have encountered this when checking out lab in Safari Private Browsing mode:

Looks like this is a feature.
I generally run my notebooks (lab/classic) in their own private browser for performance and security reasons, so I'm not banking on localStorage working for _too_ long... but hard crash is no fun.
I guess we could wrap window.localStorage (only referenced in that StateDB anyway) with an in-memory Map or something... or do the aggressive monkeypatching suggested in the SO... will try out a bit locally.
This has long been a thorn in the side of people developing for Safari. I have to admit that my immediate reaction is: well don't use private mode in Safari (and I say this as somebody who does exactly that), but yeah it isn't super high cost to have some fallback.
Heh, I don't even generally use Safari, but there you go. This (and the Ancient Browser Testing) raises a flock of questions around capability testing, which could become a coreutil in its own right!
So a StateDB is only writing to one namespace. If it were a Map, things like clear and fetchNamespace wouldn't require looking at every key, and the namespace could be dropped on the floor.
One approach would be for StateDB's constructor to sniff for the capability, and have it's own, private localStorage/dummyStorage strategy, moving the bits that touch localStorage.length and .key into the localStorage-specific implementation:
interface INamespacedStorage {
persistent: boolean;
clear(namespace: string): void;
getItem(namespace: string, key: string): string;
setItem(namespace: string, key: string, value: string): string;
getItems(namespace: string): string[];
}
class StateDB {
constructor(options: StateDB.IOptions) {
this.namespace = options.namespace;
try {
this._storage = new NamespacedLocalStorage();
this._storage.setItem(this.namespace, 'TEST', 'TEST');
} catch (err) {
this._storage = new DummyStorage();
}
}
private _storage: INamespacedStorage;
}
Actually,
INamespacedStoragecould take on even more of the complexity, and acceptJSONstuff instead of strings for values... a Map could hold the actual objects, for example.
The other would be to have a whole separate StateDB implementation, and do the sniffing at plugin activation time... however, if StateDB is advertised as something reusable, an extension author would end up having to do the sniffing themselves or run into this problem mysteriously.
I know at _some point_, there was a plan to make this potentially be server-persistable, wasn't sure which direction made the most sense.
Safari 11.0.1 fixes this issue!
Most helpful comment
Safari 11.0.1 fixes this issue!