davidmarkclements / fast-safe-stringify

Safely and quickly serialize JavaScript objects
MIT License
348 stars 27 forks source link

Uncaught TypeError: Cannot assign to read only property 'window' of object '#<Window>' #32

Closed binarykitchen closed 6 years ago

binarykitchen commented 6 years ago

Me again. Seeing the above error since I started using your library for www.videomail.io

Happens on this user agent

{
 "userAgent": "Mozilla/5.0 (Windows NT 6.3; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/64.0.3282.186 Safari/537.36"
}

I think this has been thrown at this or near this line within the decirc fn https://github.com/davidmarkclements/fast-safe-stringify/blob/master/index.js#L28

BridgeAR commented 6 years ago

Can you please show what code you executed?

binarykitchen commented 6 years ago

here, thrown when i want to stringify event.path of an unhandled rejection error in the browser for error reporting:

window.addEventListener('unhandledrejection', function (event) {
  var message = 'Unhandled rejection'
  var explanation

  ...

  if (event.path) {
    explanation += ',\npath:\n' + stringify(event.path) // <---- boooom!
  }

  ...
})
BridgeAR commented 6 years ago

It seems like replacing the value is in general not a good idea when it comes to read only properties.

E.g.

const obj = {}
Object.defineProperty(obj, 'circ', { value: obj, enumerable: true })
stringify(obj)
// Circular error
BridgeAR commented 6 years ago

As a side note: this has never worked before. This does work with https://github.com/BridgeAR/safe-stable-stringify but I guess the stable part is not necessary here.

binarykitchen commented 6 years ago

so you suggesting me to pick safe-stable-stringify instead for my use case?

binarykitchen commented 6 years ago

ping?

davidmarkclements commented 6 years ago

So this seems to be browser api specific, it won't throw for "normal" non writable properties.

We can look at creating a browser version, that detects for things like window so it doesnt throw. It will definitely need to be a separate browser entry point because we don't want to penalise Node environments - especially when fast stringification of lots of objects is common case.

In the mean time, you could see if safe-json-stringify works for your case.

On Thu, 8 Mar 2018 at 22:23, Michael Heuberger notifications@github.com wrote:

ping?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/davidmarkclements/fast-safe-stringify/issues/32#issuecomment-371628954, or mute the thread https://github.com/notifications/unsubscribe-auth/ABIrPGQOezh6taOMYAdM6Cl1UajO4DrQks5tcaEvgaJpZM4SXccD .

binarykitchen commented 6 years ago

yeah will switch to the safe-json stringifier

right, would be good to have a browser version of your lib here

davidmarkclements commented 6 years ago

closing for now