Reactotron: An object with `writable: false` prop in a state breaks SagaMonitor

[cbrwizard]

We've recently added reactotron to our project and it's pretty cool.
We've noticed though that it raises an unhandled exception on read-only object' props in sagas and actions attributes.

In this line https://github.com/infinitered/reactotron/blob/f49760fef66a24bd8ab26dd06c570176f0619c0f/packages/reactotron-core-client/src/scrub.js#L46

object[key] = getFunctionName(value);

an error is raised Cannot assign to read only property 'replace' of object. key === 'replace'.

I've noticed that it happens only when an object's key is read-only.

Object.getOwnPropertyDescriptor(object, key)
// result:
{
  configurable: false,
  enumerable: true,
  value: '',
  writable: false
}

Why do we have a read-only value in an object is a good question and we definitely need to change that, but as a quicker solution we would love to have reactotron not break our app [:

I can add a fork where we run

object[key] = getFunctionName(value);

only when

Object.getOwnPropertyDescriptor(object, key).writable

is true. Will that solve the issue?

[skellock]

Agreed... Reactotron needs to take the hippocratic oath.

Hey, so can you turn off that scrubbing? In your ReactotronConfig.js, add this in:

Reactotron.configure({ safeRecursion: false })

This turns off that scrubbing behaviour.

If you're using react-native-router-flux, logging react components with console.tron.log(), or other things with circular references, you'll have some issues.

Can you give that a shot?

[burabure]

I can also see that replaceValuesWithSomethingWeCanSeeOnTheOtherSideOfSerialization failing on other cases, I really think we ought to protect that logic with try/catch and provide safe error handling

[burabure]

@skellock turning safe-recursion off caused the stack to blow inside an action:

error RangeError: Maximum call stack size exceeded
    at _hasBinary (eval at <anonymous> (apps.js:2071), <anonymous>:29:37)
    at _hasBinary (eval at <anonymous> (apps.js:2071), <anonymous>:38:15)
    at _hasBinary (eval at <anonymous> (apps.js:2071), <anonymous>:49:63)
    at _hasBinary (eval at <anonymous> (apps.js:2071), <anonymous>:49:63)
    at _hasBinary (eval at <anonymous> (apps.js:2071), <anonymous>:49:63)
    at _hasBinary (eval at <anonymous> (apps.js:2071), <anonymous>:38:15)
    at _hasBinary (eval at <anonymous> (apps.js:2071), <anonymous>:49:63)
    at _hasBinary (eval at <anonymous> (apps.js:2071), <anonymous>:49:63)
    at _hasBinary (eval at <anonymous> (apps.js:2071), <anonymous>:49:63)
    at _hasBinary (eval at <anonymous> (apps.js:2071), <anonymous>:38:15)

PS: @cbrwizard and I work on the same codebase

[skellock]

I agree 100%. Not only that, the function is incredibly slow. I have a client with a 6mb JSON object, and it noticably slows down everything.

[skellock]

Is the action logging something like a react component? Or perhaps react-native-router-flux?

[skellock]

That stack overflow is something in socket.io (my nemesis). I can't rely on it to serialize the messages, so that scrub functionality exists to "prep" it for transport.

I've been searching for a better way to serialize that message over the wire. Everything I find and plug in has issues.

[burabure]

@skellock

[burabure]

we'd be completely fine with losing some detail on the reactotron logging if the tradeoff is not blowing up our app; do you think there's a quick patch for this?

[skellock]

Gotcha. Somewhere in _events, there's a reference to itself or another object with a circular reference.

One option:

If it's just INIT_DATA/SET causing grief, you can turn that off in a filter in your ReactotronConfig.js in the redux plugin using except.

Another thing we could do is provide a pre-serialize hook inside that same plugin. onBeforeSerialize where you had an opportunity filter out sensitive (or in your case, broken) stuff.

[burabure]

thanks @skellock I'll try that.

about the serializer, do we absolutely need to mutate the original?, how about creating a new object copy, if that's too slow, maybe just do that when mutating fails?

[skellock]

Definitely should not be mutating the original. That's a bug for sure. I'll fix that up.

[skellock]

Ugh... with a comment like that, I can't even claim it was an accident either.

[burabure]

@skellock I wasn't gonna say anything about that comment ;D

[skellock]

Here's where i went off the rails. Clone is shallow. Blargh.

[burabure]

@skellock yeah, clone is a bit weird since it does make a deep copy of everything but functions; Functions are referenced directly on the new object.

I assume this is because Ramda intends on a cloned object to be deepEqual to the original

[skellock]

Gotcha. Thx.

What i need is something that recursively walks a structure. Each visit calling an optional callback allowing me to override the value placed inside.

Like this but for JSON and a little more open to callbacks. Preferably a dependency-free package that I didn't write either. =)

[burabure]

@skellock recursive-iterator might get the job done.

or one off this list should be able to replace clone from ramda, deep-copy looks fast and simple

[skellock]

Sweet @burabure . Thx for the refs!

[burabure]

@skellock we ended up disabling reactotron for now ;___; sadly after applying the quick fixes (disable recursion and ignore recursive action stuff) it silently crashed some of our sagas/actions. I hope a fix for this goes up soon because we loved debugging with reactotron

[skellock]

Thx for letting me know.

Reactotron is back at the top of my list of priorities now that I've finished gluegun and wrapping up ignite 2.

Keep ya posted.

[skellock]

Fixed in 1.9. Shipping tomorrow.

[burabure]

@skellock it's working perfectly now! thanks!