cartant / rxjs-spy

A debugging library for RxJS
https://cartant.github.io/rxjs-spy/
MIT License
702 stars 22 forks source link

Crash browser tab while creating spy instance in React 16.0.0 #6

Closed vshy108 closed 6 years ago

vshy108 commented 6 years ago

rxjs-spy: 5.0.0 react: 16.0.0

I tried to include following lines in one of my redux ducks file.

import { spy } from 'rxjs-spy';
spy();

and it always crash the browser tab after long time loading.

vshy108 commented 6 years ago

May I know what type of information should I provide?

cartant commented 6 years ago

You've specified the versions that you are using, so thanks for that.

However, just saying that it crashes is too general. The issue needs to include an error message and some easily reproducible steps to create a project that exhibits the bug.

For more guidelines on submitting a bug report, see How to Report Bugs Effectively

cartant commented 6 years ago

Also, if you've not already tried it, you could try disabling the graphing and snapshotting plugins:

spy({ plugins: [] });

If that solves the problem, I'd still appreciate your including and error message and any steps to reproduce the problem with a new project.

vshy108 commented 6 years ago

Operating System: macOS Sierra 10.12.6 Browser: Google chrome version 62.0.3202.75 rxjs-spy: 5.0.0 react: 16.0.0

Reproduce step: 1) yarn add rxjs-spy 2) Add following line to any of redux ducks file that have observable epics.

import { spy } from 'rxjs-spy';
spy();

3) Refresh app 4) Chrome tab is freezing

Error image: image

Further info

package.json

{
  "name": "",
  "version": "0.1.0",
  "private": true,
  "engines": {
    "node": "8.7.0",
    "npm": "5.3.0",
    "yarn": "1.2.1"
  },
  "dependencies": {
    "classname": "^0.0.0",
    "d3": "^4.10.0",
    "geodesy": "^1.1.2",
    "google-maps": "^3.2.1",
    "history": "^4.6.3",
    "idx": "^2.2.0",
    "js-file-download": "^0.4.1",
    "material-components-web": "^0.23.0",
    "moment": "^2.19.1",
    "parse": "^1.10.1",
    "rc-pagination": "^1.12.10",
    "react": "^16.0.0",
    "react-color": "^2.13.4",
    "react-dom": "^16.0.0",
    "react-google-maps": "^9.2.1",
    "react-modal": "^3.1.0",
    "react-network": "^1.0.1",
    "react-notification-system": "^0.2.16",
    "react-redux": "^5.0.6",
    "react-redux-form": "^1.15.0",
    "react-router-dom": "^4.1.2",
    "redux": "^3.7.2",
    "redux-logger": "^3.0.6",
    "redux-observable": "^0.16.0",
    "redux-persist": "^5.1.0",
    "redux-thunk": "^2.2.0",
    "reselect": "^3.0.1",
    "rxjs": "^5.5.2",
    "rxjs-spy": "^5.0.0",
    "save-svg-as-png": "^1.2.0",
    "sha1": "^1.1.1",
    "validator": "^9.1.1"
  },
  "devDependencies": {
    "babel-eslint": "^8.0.1",
    "eslint": "^4.10.0",
    "eslint-config-airbnb": "^16.1.0",
    "eslint-config-prettier": "^2.6.0",
    "eslint-plugin-flowtype": "^2.35.0",
    "eslint-plugin-import": "^2.7.0",
    "eslint-plugin-jsx-a11y": "^6.0.2",
    "eslint-plugin-react": "^7.4.0",
    "flow-bin": "^0.56.0",
    "prettier": "^1.7.4",
    "react-scripts": "^1.0.11"
  },
  "scripts": {
    "start": "react-scripts start",
    "build": "react-scripts build",
    "test": "react-scripts test --env=jsdom",
    "eject": "react-scripts eject",
    "lint": "eslint src/",
    "flow": "flow"
  }
}

configureStore.js

// @flow
import { applyMiddleware, createStore, compose } from 'redux';
import { createLogger } from 'redux-logger';
import { createEpicMiddleware } from 'redux-observable';
import thunk from 'redux-thunk';
import storage from 'redux-persist/es/storage'; // default: localStorage if web, AsyncStorage if react-native
import { persistStore, persistReducer } from 'redux-persist';

import rootEpic from './epics';
import reducers, { persistentStoreBlacklist } from './reducers';

const epicMiddleware = createEpicMiddleware(rootEpic);

const logger = createLogger({
  collapsed: true,
  duration: true,
});

export default function configureStore() {
  let enhancer;
  if (process.env.NODE_ENV === 'development') {
    enhancer = compose(applyMiddleware(logger, thunk, epicMiddleware));
  } else {
    enhancer = compose(applyMiddleware(thunk, epicMiddleware));
  }
  const config = {
    key: 'root', // key is required
    storage, // storage is now required
    blacklist: persistentStoreBlacklist,
  };
  const reducer = persistReducer(config, reducers);
  const store = createStore(reducer, enhancer);
  const persistor = persistStore(store);
  return { store, persistor };
}
vshy108 commented 6 years ago

spy({ plugins: [] });

solves the problem. But it won't inspect the current snapshots of all tagged observables because disabling the graphing and snapshotting plugins?

cartant commented 6 years ago

With those plugins not present, show won't work, but you can still use log to see the notifications for tagged observables. It sounds like you have a large number of notifications happening and the default settings for spy are not appropriate.

The snapshotting implementation flushes unsubscribed snapshots, but only after a delay - as it's sometimes useful to look at closed subscriptions. By default, unsubscribed snapshots are kept for 30 seconds and it sounds like that is far too long for your app. Try these options:

spy({ keptDuration: 0, keptValues: 1 });

That should see snapshots for unsubscribed subscriptions flushed immediately and the snapshots themselves should hold only one value (they default to holding the most recent four values).

If you still have a problem, it's possible/likely that I have a bug in the flushing code, so you should revert to disabling the default plugins.

If tweaking the kept setting makes a difference, I guess I should probably do something to dynamically alter the default to something sensible if there are a lot of unsubscribes happening in a short period of time.

vshy108 commented 6 years ago

spy({ keptDuration: 0, keptValues: 1 });

will causing same freezing problem in my code.

I revert to the option disabling the default plugin and use log to see the notifications for tagged observables successfully.

In my case, I only tested the functionality with only one tag when use is logging in. Hence, it should not have large number of notifications. Furthermore, the freezing problem happens even though it is creating spy instance without any tag.

Thanks for fast response and suggestion. Happy to know got such a tool to spy the observable streams.

cartant commented 6 years ago

Good to hear that it's working.

Regarding the removal of the default plugins, it really only effects the snapshotting and show. You can still use log, pause, and let which add tag-specific plugins when the methods themselves are called.

I'm going to add a simple StatsPlugin to tally the number of notifications, etc., as I'd like the snapshotting to be as lightweight as possible. If it's not working for your app, I'd like to fix it.

Once I've added the plugin (I'll let you know when), if you could include it and provide me with the results, I'd appreciate it. I'd also appreciate any information you can provide relating to how many subscriptions, unsubscriptions and notifications occur in your app (in the time is usually takes spy to crash your tab).

spy hooks into the RxJS implementation at a very low level. It sees everything regardless of whether tag is used or not. The tag is just for identification purposes.

Thanks for raising the issue.

vshy108 commented 6 years ago

Thanks for your efforts, looking forward to the StatsPlugin. My app has around 52 observables, if the spysees everything regardless of whether tag is used or not, then it is really very busy in my app.

At login page of my app, there only have persist and rehydrate action of redux-persist dispatched but spy() will still crash the app somehow.

I tried to configure the spy with following spy({ plugins: [], keptDuration: -1, keptValues: 10, warning: true }); but then I found that the keptValues option is no effect in my app because whether I change it to 1 or 4 or 10, my apps always show maximum 4 notifications of next even though it should show more.

May I know how to add specified plugin in the plugins array option? May be i can further debug the crash by adding plugin per plugin?

vshy108 commented 6 years ago

After I turned on the pause on exception on Sources tab in chrome's debugging tool, I found the crash maybe caused by this line

spy.js line 445 plugins_.forEach(function (plugin) { return plugin.beforeSubscribe(ref); });

image

The Error StackTrace on that line is

Error at StackTrace$$GenerateError (http://localhost:3000/static/js/bundle.js:184634:19) at Object.StackTrace$$getSync [as getSync] (http://localhost:3000/static/js/bundle.js:184696:23) at StackTracePlugin../node_modules/rxjs-spy/plugin/stack-trace-plugin.js.StackTracePlugin.beforeSubscribe (http://localhost:3000/static/js/bundle.js:147554:41) at http://localhost:3000/static/js/bundle.js:148036:56 at Array.forEach () at Subject.subscribeWithSpy [as subscribe] (http://localhost:3000/static/js/bundle.js:148036:14) at ActionsObservable../node_modules/rxjs/Observable.js.Observable._subscribe (http://localhost:3000/static/js/bundle.js:148664:28) at ActionsObservable../node_modules/rxjs/Observable.js.Observable.subscribe (http://localhost:3000/static/js/bundle.js:148593:41) at ActionsObservable.subscribeWithSpy [as subscribe] (http://localhost:3000/static/js/bundle.js:148037:38) at FilterOperator../node_modules/rxjs/operators/filter.js.FilterOperator.call (http://localhost:3000/static/js/bundle.js:165712:23) at ActionsObservable../node_modules/rxjs/Observable.js.Observable.subscribe (http://localhost:3000/static/js/bundle.js:148590:22) at ActionsObservable.subscribeWithSpy [as subscribe] (http://localhost:3000/static/js/bundle.js:148037:38) at SwitchFirstMapOperator../node_modules/rxjs/operators/exhaustMap.js.SwitchFirstMapOperator.call (http://localhost:3000/static/js/bundle.js:165404:23) at ActionsObservable../node_modules/rxjs/Observable.js.Observable.subscribe (http://localhost:3000/static/js/bundle.js:148590:22) at ActionsObservable.subscribeWithSpy [as subscribe] (http://localhost:3000/static/js/bundle.js:148037:38) at Object.subscribeToResult (http://localhost:3000/static/js/bundle.js:175639:27) at MergeMapSubscriber../node_modules/rxjs/operators/mergeMap.js.MergeMapSubscriber._innerSub (http://localhost:3000/static/js/bundle.js:167540:38) at MergeMapSubscriber../node_modules/rxjs/operators/mergeMap.js.MergeMapSubscriber._tryNext (http://localhost:3000/static/js/bundle.js:167537:14) at MergeMapSubscriber../node_modules/rxjs/operators/mergeMap.js.MergeMapSubscriber._next (http://localhost:3000/static/js/bundle.js:167520:18) at MergeMapSubscriber../node_modules/rxjs/Subscriber.js.Subscriber.next (http://localhost:3000/static/js/bundle.js:149553:18) at Object.next (http://localhost:3000/static/js/bundle.js:147937:24) at SafeSubscriber../node_modules/rxjs/Subscriber.js.SafeSubscriber.tryOrSetError (http://localhost:3000/static/js/bundle.js:149711:16) at SafeSubscriber../node_modules/rxjs/Subscriber.js.SafeSubscriber.next (http://localhost:3000/static/js/bundle.js:149651:27) at Subscriber../node_modules/rxjs/Subscriber.js.Subscriber._next (http://localhost:3000/static/js/bundle.js:149589:26) at Subscriber../node_modules/rxjs/Subscriber.js.Subscriber.next (http://localhost:3000/static/js/bundle.js:149553:18) at Object.next (http://localhost:3000/static/js/bundle.js:147993:40) at SafeSubscriber../node_modules/rxjs/Subscriber.js.SafeSubscriber.__tryOrSetError (http://localhost:3000/static/js/bundle.js:149711:16) at SafeSubscriber../node_modules/rxjs/Subscriber.js.SafeSubscriber.next (http://localhost:3000/static/js/bundle.js:149651:27) at Subscriber../node_modules/rxjs/Subscriber.js.Subscriber._next (http://localhost:3000/static/js/bundle.js:149589:26) at Subscriber../node_modules/rxjs/Subscriber.js.Subscriber.next (http://localhost:3000/static/js/bundle.js:149553:18) at ArrayObservable../node_modules/rxjs/observable/ArrayObservable.js.ArrayObservable._subscribe (http://localhost:3000/static/js/bundle.js:152338:28) at ArrayObservable../node_modules/rxjs/Observable.js.Observable._trySubscribe (http://localhost:3000/static/js/bundle.js:148605:25) at ArrayObservable../node_modules/rxjs/Observable.js.Observable.subscribe (http://localhost:3000/static/js/bundle.js:148593:65) at ArrayObservable.subscribeWithSpy [as subscribe] (http://localhost:3000/static/js/bundle.js:148037:38) at MergeMapOperator../node_modules/rxjs/operators/mergeMap.js.MergeMapOperator.call (http://localhost:3000/static/js/bundle.js:167495:23) at Observable../node_modules/rxjs/Observable.js.Observable.subscribe (http://localhost:3000/static/js/bundle.js:148590:22) at Observable.subscribeWithSpy [as subscribe] (http://localhost:3000/static/js/bundle.js:148037:38) at Object.subscribeToResult (http://localhost:3000/static/js/bundle.js:175639:27) at MergeMapSubscriber../node_modules/rxjs/operators/mergeMap.js.MergeMapSubscriber._innerSub (http://localhost:3000/static/js/bundle.js:167540:38) at MergeMapSubscriber../node_modules/rxjs/operators/mergeMap.js.MergeMapSubscriber._tryNext (http://localhost:3000/static/js/bundle.js:167537:14) at MergeMapSubscriber../node_modules/rxjs/operators/mergeMap.js.MergeMapSubscriber._next (http://localhost:3000/static/js/bundle.js:167520:18) at MergeMapSubscriber../node_modules/rxjs/Subscriber.js.Subscriber.next (http://localhost:3000/static/js/bundle.js:149553:18) at Object.next (http://localhost:3000/static/js/bundle.js:147937:24) at SafeSubscriber../node_modules/rxjs/Subscriber.js.SafeSubscriber.tryOrSetError (http://localhost:3000/static/js/bundle.js:149711:16) at SafeSubscriber../node_modules/rxjs/Subscriber.js.SafeSubscriber.next (http://localhost:3000/static/js/bundle.js:149651:27) at Subscriber../node_modules/rxjs/Subscriber.js.Subscriber._next (http://localhost:3000/static/js/bundle.js:149589:26) at Subscriber../node_modules/rxjs/Subscriber.js.Subscriber.next (http://localhost:3000/static/js/bundle.js:149553:18) at Object.next (http://localhost:3000/static/js/bundle.js:147993:40) at SafeSubscriber../node_modules/rxjs/Subscriber.js.SafeSubscriber.__tryOrSetError (http://localhost:3000/static/js/bundle.js:149711:16) at SafeSubscriber../node_modules/rxjs/Subscriber.js.SafeSubscriber.next (http://localhost:3000/static/js/bundle.js:149651:27)

cartant commented 6 years ago

That looks like it's coming from stacktrace-js. You can pass sourceMaps: false to the spy call to disable source map resolution for stack traces. It's possible that high-frequency notifications are a problem for stacktrace-js.

cartant commented 6 years ago

Actually, that's probably a false alarm, as stacktrace-js will throw and catch an error to determine the stack trace. However, disabling source map resolution would be something to try anyway.

The other thing you can try, if you are up for experimenting is to specify the graph and snapshot plugins yourself. For example, to use only the GraphPlugin you could do this:

import { spy } from "rxjs-spy";
import { GraphPlugin } from "rxjs-spy/plugin";
spy({ plugins: [new GraphPlugin()] });

It would be interesting to see if it still crashes without either the StackTracePlugin or SnapshotPlugin. By itself, the GraphPlugin won't do anything useful, but trying with it alone might help find the culprit.

vshy108 commented 6 years ago

spy({ sourceMaps: false }); is working fine.

I try to add the plugins one by one and found that,

StackTracePlugin is the one cause freezing but not the others. spy({ plugins: [new StackTracePlugin())] }); freezes. spy({ plugins: [new GraphPlugin(), new SnapshotPlugin()] }); works fine.

cartant commented 6 years ago

Cool. Thanks for the information.

cartant commented 6 years ago

rxjs-spy 5.1.1 now has a very basic stats plugin. If you type rxSpy.stats() into the console, it will log the notification tallies (if the StatsPlugin has been specified).

Also, the sourceMaps option now defaults to false. I'm going to have to re-think how the source map resolution works, as resolving the source maps for each notification clearly has problems.

vshy108 commented 6 years ago

The attached image is the basic stats result of my code.

image

The subscribers and nexts have quite large number right?

cartant commented 6 years ago

Thanks. That should be really helpful in deciding how to improve the handling of source maps.

If you have the time to repeat your stats gathering with a new version, that would be great, as I've updated the StatsPlugin in version 5.2.0 to include some graph-related stats:

They are only included if the GraphPlugin is configured.

vshy108 commented 6 years ago

Stats result from 5.2.0

image

cartant commented 6 years ago

Thanks. That should be helpful.

cartant commented 6 years ago

I notice that you've just forked this repo. FYI, last night I started looking into this issue. I'm currently looking into what effects the massive memory usage when snapshotting and source map resolution is configured. If you, too, are looking into this, any information you have will be received with thanks.

cartant commented 6 years ago

FYI. As of version 6.1.0, the sourceMaps option should no longer use massive amounts of memory.