PillowPillow / ng2-webstorage

Localstorage and sessionstorage manager - Angular service
MIT License
428 stars 91 forks source link

Unhandled exception thrown from getLocalStorage() #111

Open stretch1975 opened 5 years ago

stretch1975 commented 5 years ago

Versions (please complete the following information):

Describe the bug Some IE 11 settings can result in an error that says, "The operations attemtped to access data outside the valid range" when trying to get the value for window.localStorage.

To Reproduce Steps to reproduce the behavior:

  1. Add ngx-webstorage to your project and browse to it in IE

Expected behavior The exception should be caught and handled gracefully

Screenshots The attached screenshot shows how the window.localStorage value can throw an exception and how other developers have handled it... ngx-webstorage exception

Desktop (please complete the following information):

Additional context Locally if I change this code: function getLocalStorage() { return (typeof window !== 'undefined') ? window.localStorage : null;
}

To this: function getLocalStorage() { try{ return (typeof window !== 'undefined') ? window.localStorage : null; }catch(err){ return null; } }

It will work

Thanks!

bRRRITSCOLD commented 5 years ago

This has been bugging our team for months on end too! We havent been able to pinpoint it until now.

Making the below changes, just like the OP, fixed my issues:

Source Code:

function getLocalStorage() {
  return (typeof window !== 'undefined') ? window.localStorage : null;
}

Change:

function getLocalStorage() {
  try{
    return (typeof window !== 'undefined') ? window.localStorage : null;
  } catch (err) {
    return null;
  }
}

However, I am unsure how the community would react to catching errors and always returning null, my proposed change is below:

function getLocalStorage() {
   return (typeof window !== 'undefined' && window.localStorage) ? window.localStorage : null
}

I think this could bleed over to getSessionStorage. My proposed change to that - while we are at it - would look like:

function getSessionStorage() {
   return (typeof window !== 'undefined' && window.sessionStorage) ? window.sessionStorage : null
}

Thoughts?

PillowPillow commented 5 years ago

Hello ! thanks for this issue.

I'm unable to test on IE till next week but the fix seems weird to me. How the localstorage could be unavailable if window exists..? Does this issue occurs on every setups ? not just on private mode or so ?

stretch1975 commented 5 years ago

This appears to be a common issue with IE running in protected mode. See this link for another package that dealt with the same issue: https://github.com/tinymce/tinymce/issues/4338

Sent from my iPhone

On Jun 13, 2019, at 5:00 AM, Nicolas Gaignoux notifications@github.com wrote:

Hello ! thanks for this issue.

I'm unable to test on IE till next week but the fix seems weird to me. How the localstorage could be unavailable if window exists..? Does this issue occurs on every setups ? not just on private mode or so ?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

bRRRITSCOLD commented 5 years ago

Pillow - looks like the comment in stretch's linked issue that you need to look at to recreate:

nileshbhagwat's comment on Apr 13, 2018

psalkowski commented 5 years ago

I have similar problem, localStorage is not always available. Some users can disable local storage (and session storage) in browser settings. In that case, the above error will be thrown.

My suggestion is to make local storage in object (key->value). It will work until user will refresh page, but should be enough.

Also, if you just use window.localStorage when it's disabled, it will thrown an exception (access denied), so, the test should be different:

try {
    window.localStorage.getItem('test');
    return window.localStorage;
} catch(ex) {
    return null;
}

Same thing for sessionStorage

ezpar commented 4 years ago

This issue still seems to persist even in the most current 5.0.0 version. Any chance you could implement one of the suggested solutions?

vixriihi commented 3 years ago

This is also still an issue in 6.0.0. This also occurs when loading a page in an iframe and when in Chrome 3rd party cookies are not allowed (seems like localstorage and sessionstorage are not available then either and throw an exception when accessed). We had to wrap both getLocalStorage getSessionStorage with a try catch mentioned in bRRRITSCOLDs comment.