Capture serialized errors/stack traces when actions are dispatched #21
Conversation
This is a prerequisite for displaying stack traces in the DevTools Extension UI.
|
Okay, apparently linting matters. I'll fix that this evening. |
|
Hey @markerikson thanks for working on this. I fixed the linting, but tests still fail as stacks on importing actions don't match. One of the tests as example: - "stack": "Error\n at Object.performAction (/home/travis/build/zalmoxisus/redux-devtools-instrument/src/instrument.js:43:101)\n at liftAction (/home/travis/build/zalmoxisus/redux-devtools-instrument/src/instrument.js:191:25)\n at /home/travis/build/zalmoxisus/redux-devtools-instrument/src/instrument.js:490:32\n at Object.dispatch (/home/travis/build/zalmoxisus/redux-devtools-instrument/node_modules/redux/lib/redux.js:211:22)\n at Context.<anonymous> (/home/travis/build/zalmoxisus/redux-devtools-instrument/test/instrument.spec.js:702:34)\n at callFn (/home/travis/build/zalmoxisus/redux-devtools-instrument/node_modules/mocha/lib/runnable.js:326:21)\n at Test.Runnable.run (/home/travis/build/zalmoxisus/redux-devtools-instrument/node_modules/mocha/lib/runnable.js:319:7)\n at Runner.runTest (/home/travis/build/zalmoxisus/redux-devtools-instrument/node_modules/mocha/lib/runner.js:422:10)\n at /home/travis/build/zalmoxisus/redux-devtools-instrument/node_modules/mocha/lib/runner.js:528:12\n at next (/home/travis/build/zalmoxisus/redux-devtools-instrument/node_modules/mocha/lib/runner.js:342:14)\n at /home/travis/build/zalmoxisus/redux-devtools-instrument/node_modules/mocha/lib/runner.js:352:7\n at next (/home/travis/build/zalmoxisus/redux-devtools-instrument/node_modules/mocha/lib/runner.js:284:14)\n at /home/travis/build/zalmoxisus/redux-devtools-instrument/node_modules/mocha/lib/runner.js:315:7\n at done (/home/travis/build/zalmoxisus/redux-devtools-instrument/node_modules/mocha/lib/runnable.js:287:5)\n at callFn (/home/travis/build/zalmoxisus/redux-devtools-instrument/node_modules/mocha/lib/runnable.js:344:7)\n at Hook.Runnable.run (/home/travis/build/zalmoxisus/redux-devtools-instrument/node_modules/mocha/lib/runnable.js:319:7)\n at next (/home/travis/build/zalmoxisus/redux-devtools-instrument/node_modules/mocha/lib/runner.js:298:10)\n at Immediate._onImmediate (/home/travis/build/zalmoxisus/redux-devtools-instrument/node_modules/mocha/lib/runner.js:320:5)\n at tryOnImmediate (timers.js:534:15)\n at processImmediate [as _immediateCallback] (timers.js:514:5)"
+ "stack": "Error\n at Object.performAction (/home/travis/build/zalmoxisus/redux-devtools-instrument/src/instrument.js:43:101)\n at liftAction (/home/travis/build/zalmoxisus/redux-devtools-instrument/src/instrument.js:191:25)\n at liftReducerWith (/home/travis/build/zalmoxisus/redux-devtools-instrument/src/instrument.js:201:23)\n at liftReducer (/home/travis/build/zalmoxisus/redux-devtools-instrument/src/instrument.js:668:14)\n at /home/travis/build/zalmoxisus/redux-devtools-instrument/src/instrument.js:671:37\n at createStore (/home/travis/build/zalmoxisus/redux-devtools-instrument/node_modules/redux/lib/redux.js:85:33)\n at Context.<anonymous> (/home/travis/build/zalmoxisus/redux-devtools-instrument/test/instrument.spec.js:690:24)\n at callFn (/home/travis/build/zalmoxisus/redux-devtools-instrument/node_modules/mocha/lib/runnable.js:326:21)\n at Hook.Runnable.run (/home/travis/build/zalmoxisus/redux-devtools-instrument/node_modules/mocha/lib/runnable.js:319:7)\n at next (/home/travis/build/zalmoxisus/redux-devtools-instrument/node_modules/mocha/lib/runner.js:298:10)\n at Immediate._onImmediate (/home/travis/build/zalmoxisus/redux-devtools-instrument/node_modules/mocha/lib/runner.js:320:5)\n at tryOnImmediate (timers.js:534:15)\n at processImmediate [as _immediateCallback] (timers.js:514:5)"I'll look into that. If you have any ideas on how to preserve it, let me know. |
|
Hmm. Hadn't thought about the importing case. Which test, and can you give an example of how to repro the issue? |
|
I'm trying to understand the necessity of added |
|
@markerikson can find it in travis error. Just run |
|
I pulled that code from https://github.com/sindresorhus/serialize-error , on the grounds that I wanted to make sure it got serialized properly. Maybe it wasn't necessary, but it was pretty much the first thing that came up when I searched for something like "js serialize error" or something like that :) |
|
Did you by any chance see if throwing an error on every action would decrease the performance significantly? Some applications dispatching lots of actions frequently, and could have issues if the performance would decrease much, especially that the extension is autoupdated automatically. Usually it doesn't affect the performance if not opened. In that case we should make this optional and not enabled by default. |
|
No, I didn't do any perf checking. FWIW, it's just creating It looks like the test failures are because the stack traces are being captured at the time the action is recorded, so the stack from a dispatch is different from the stack from an import, and therefore the old and new states are different. Also not sure what a good way is to handle that. |
|
Yep, it probably depends how fast Chrome is getting stack trace internally. Which would depend on every individual case, if it's deep inside with many functions and files. |
|
Ideally the monitor part would emit an action to generate the stack trace, then instrumentation part would recompute action and send the stack on request. But it would be more complicated. Probably easier, just to make it optional and mentioning in that tab that it should be added as parameter if needed. At least as the first step and not having regression in the performance. |
|
Alright, so even though Error.stack is not standard, according to the specification it's string in Chrome and Firefox. Microsoft specs redirects to the same page so we could assume it should be string there too. Checked in Safari and it's also string. Will add it as an option as some others. If we can check the performance and it have no regressions, can make default in feature versions. As per tests, we can just mention that it doesn't work on importing. We are not dispatching actions again, so not possible to make it work. |
|
Actually, that's good, when we're importing we want to see the stack trace of the person who exported, not our stack trace. So it does that. |
|
I published it as |
performAction()now creates anErrorobject and serializes it as a field in the lifted action. This is a prerequisite for displaying stack traces in the DevTools Extension UI.See zalmoxisus/redux-devtools-extension#429 for the original request.