performance.mark is not always available

[Amatewasu]

Despite having a good adoption by the current browsers, sometimes performance.mark is still unavailable in some environments.

In my case, it is not available in my test environment leading to a crash of some of my units tests. It looks like it is due to the fake timers of Jest that remove the performance.mark function. (I wanted to create a codesandbox to show the problem but we cannot use the jest fake timers in codesandbox).

It would be nice to check if performance.mark really exists before calling it and therefore avoid raising an error.

For example, here's the error I get:

Test suite failed to run

    TypeError: performance.mark is not a function

      40 | epicMiddleware.run(rootEpic);
      41 |
    > 42 | store.dispatch(initAction());
         |       ^
      43 |
      44 | export default store;
      45 |

      at performProfiledOperation (node_modules/redux-profiler/dist/redux-profiler.umd.js:145:21)
      at Object.dispatch (node_modules/redux-profiler/dist/redux-profiler.umd.js:203:21)
      at Object.<anonymous> (src/store/index.ts:42:7)
      at Object.<anonymous> (src/components/XanonymizationX.test.tsx:6:1)

What do you think? (if needed, I can send a PR)

[bhovhannes]

PRs are very welcome!

There already is a check in the code - https://github.com/bhovhannes/redux-profiler/blob/main/src/index.js#L122-L124

Perhaps additional checks can be added to these lines - https://github.com/bhovhannes/redux-profiler/blob/main/src/index.js#L42-L47, to make sure not only performance object exist, but also to make sure it contains mark function.

[bhovhannes]

I'd rather disable redux-profiler during unit tests, but there are still cases when in theory it can be useful, let's say if running jest with puppeteer to measure interaction time, that's why I still think that fixing this here is still a good idea.

[Amatewasu]

Thanks for your quick answer!

I agree with I would say it is more relevant to disable redux-profiler during the unit tests. Furthermore, it is a special case because it looks that it is jest that destroys performance.mark during the runtime.

Otherwise, I was thinking of implementing a check to these lines:

redux-profiler/src/index.js

Lines 63 to 65 in 3f3ed74

	performance.mark(markName + START)
	const res = op()
	performance.mark(markName + END)

[bhovhannes]

That way checks will run for each action. Better to make redux-profiler to be a noop if performance.mark is not available.
The patch below should do that.

Index: src/index.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/index.js b/src/index.js
--- a/src/index.js	(revision 2b6863855fc8ba05c0793c4015fddbad3d6d6aeb)
+++ b/src/index.js	(date 1638188151119)
@@ -45,6 +45,10 @@
 	} else if (typeof window !== 'undefined') {
 		performance = window.performance
 	}
+	
+	if (typeof performance !== 'object' || typeof performance.mark !== 'function') {
+		performance = undefined
+	}
 
 	const performProfiledOperation = (() => {
 		let counter = {}

[Amatewasu]

Here's a digression from the original problem for those who will read this in the future and want to disable redux-profiler for their test environment.
I configured window.__TEST__ to be true in my test environment (and undefined in my other environments).

I created an enhancer that does nothing (i.e. the identity reducer):

const entityEnhancer =
  (createStore) =>
  (reducer, initialState, enhancer) => {
    const monitoredReducer = (state) => reducer(state, action);

    return createStore(monitoredReducer, initialState, enhancer);
  };

And then I create the my store with the following manner:

const store = createStore(
  rootReducer(history),
  composeEnhancers(!!window.__TEST__ ? entityEnhancer : profileStore(), enhancers)
);

[Amatewasu]

Regarding your suggestion, I agree with you that it would be better if redux-profiler is noop when the performance object is not fully available.

[bhovhannes]

I went ahead and fixed this myself as I had some unexpected free time. Sorry if you have already started to work on that.

Fix is released in version 1.0.9. Please let me know if it works.

[Amatewasu]

Thank you a lot for that. However, I have upgraded the package to 1.0.9 and enabled back redux-profiler of the unit tests of my current project and ran them but it still raised the same error as in 1.0.8.

Here's the error trace I got:

TypeError: performance.mark is not a function

      87 | epicMiddleware.run(rootEpic);
      88 |
    > 89 | store.dispatch(initAction());
         |       ^
      90 |
      91 | export default store;
      92 |

      at performProfiledOperation (node_modules/redux-profiler/dist/redux-profiler.umd.js:145:21)
      at Object.dispatch (node_modules/redux-profiler/dist/redux-profiler.umd.js:203:21)
      at Object.<anonymous> (src/store/index.ts:89:7)
      at Object.<anonymous> (src/components/layers/XmyFileAnonimizedX.test.tsx:8:1)

[Amatewasu]

In my case, it is probably Jest that destroys the performance object during the runtime. I look a bit in the code and I think if we want to prevent this from happening we need to do the check at line 73.

redux-profiler/src/index.js

Lines 72 to 91 in f33d0d0

	return function (markInfo, markType, op) {
	counter[markInfo.name] = counter[markInfo.name] === undefined ? 0 : counter[markInfo.name] + 1
	const markName = getMarkName(markInfo)
	performance.mark(markName + START)
	const res = op()
	performance.mark(markName + END)
	performance.measure(getMarkLabel(markInfo, markType), markName + START, markName + END)
	performance.clearMarks(markName + START)
	performance.clearMarks(markName + END)
	performance.clearMeasures(getMarkLabel(markInfo, markType))
	if (counter[markInfo.name]) {
	counter[markInfo.name]--
	} else {
	delete counter[markInfo.name]
	}
	return res
	}
	})()

[bhovhannes]

I don't understand what do you mean.
Code already checks for valid performance object in profileStore. If it goes beyond that point, performance is available and had all required methods.
I don't think Jest destroys performance object after profileStore is called but before some redux action happens. That does not make sense.

Can you put together a sample repo reproducing the issue?

[Amatewasu]

Sorry for the late reply. I fully agree with you, it does not make any sense and it would not be redux-profiler's job to handle such weird things.

Anyway I have set-up a small repo to try to reproduce the issue but I was not able to reproduce... ( https://github.com/Amatewasu/redux-profiler_issue-655 )
Thanks a lot for all!

[bhovhannes]

I am glad to help, but honestly I think code is defensive as it could be and I don't see any code paths which may fail if for some reason performance object is not available or has a wrong shape.

Without reproducible case I don't know how I can help.