Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TypeError: null is not an object (evaluating 'H.scale') #7705

Closed
andrejslogins opened this issue Feb 19, 2022 · 22 comments
Closed

TypeError: null is not an object (evaluating 'H.scale') #7705

andrejslogins opened this issue Feb 19, 2022 · 22 comments
Labels
needs_confirmation stale Issue marked as stale by the stale bot

Comments

@andrejslogins
Copy link
Contributor

TypeError: null is not an object (evaluating 'H.scale') error is thrown.
variable H based on stacktrace seems to be context, likely changed due to doing a build and minimizing the code.

Version

5.1.0

Information about environment

Browser
Mobile Safari 15.3
Chrome Mobile iOS 98.0.4758
Safari 15.0 (desktop)

Steps To Reproduce

No clear steps to reproduce at this point, though this has been popping up on sentry for my project's users.

Error Message & Stack Trace

TypeError: null is not an object (evaluating 'H.scale')
  at __initRetinaScaling(/build/main.4835ee4746e926683e9c.js:1:96381)
  at _initRetinaScaling(/build/main.4835ee4746e926683e9c.js:1:96227)
  at _initInteractive(/build/main.4835ee4746e926683e9c.js:1:123257)
  at initialize(/build/main.4835ee4746e926683e9c.js:1:122264)
  at T(/build/main.4835ee4746e926683e9c.js:1:38821)
  at ? (/build/main.4835ee4746e926683e9c.js:1:3957453)
  at i(/build/main.4835ee4746e926683e9c.js:1:1262561)
  at ? (/build/polyfills.a4bf941f0f8d5c4b9264.js:1:35310)
  at onInvokeTask(/build/main.4835ee4746e926683e9c.js:1:1124990)
  at runTask(/build/polyfills.a4bf941f0f8d5c4b9264.js:1:6605)
  at invokeTask(/build/polyfills.a4bf941f0f8d5c4b9264.js:1:12288)

Expected Behavior

Error should not be thrown and context.scale should be called without issues

Actual Behavior

In some rare cases context.scale is undefined

@ShaMan123
Copy link
Contributor

Again, I suspect you have a memory leak. Please be 100% sure you don't.
call fabric.runningAnimation.cancelAll() to make sure you don't (I don't know where you should call this, at cleanup of app, and before refreshing in dev mode)
If it returns something you are leaking memory.
#7694 #7693

@andrejslogins
Copy link
Contributor Author

andrejslogins commented Feb 19, 2022

@ShaMan123 I'm very confident we don't have animations in our code. Never we call .animate or using fabric.runningAnimation for the code, so I don't see where fabric.runningAnimation.cancelAll() would help.
This specific thing is occurring when we initialize canvas (therefore it's calling __initRetinaScaling). I've checked if our code tries to initialize canvas multiple times OR overwriting it, but that does not seem to be the case either.

Also, what's this runningAnimation module? :)

For me it feels like some race-condition cases. As i've described this in the ticket, it's not even properly reproducable, but i'm confident it occurs, as sentry reports us this.

@ShaMan123
Copy link
Contributor

Also, what's this runningAnimation module? :)

It's a store of all current running animations. We added that to v4.6+ so I thought you were hitting something there

@andrejslogins
Copy link
Contributor Author

This issue was in our sentry before upgrading to 4.6, so that's a no.
I can continue trying to do a proper repro, sure.

@ShaMan123
Copy link
Contributor

What is sentry?

@andrejslogins
Copy link
Contributor Author

Sentry is a tool that we integrate with our app and it forwards all client-side errors to an external service, where we can check previous interactions/other user data before the encountered error

https://sentry.io/welcome/ can check more about it here

@asturur
Copy link
Member

asturur commented Feb 20, 2022

So is safari related, mobile.
That could be an out of memory error.
Safari mobile limit the amount of memory you can use with canvases and it could be that the cache context are exhausting the memory pool.
That would cover chrome mobile too on IOS that is just a skinned safari as any other IOS browser.

The safari desktop is unexplained to me.

Do you have any stats of what is going on on the canvas before that happen?
Number of cached objects, size of the canvas, size of the object when is trying to be cached?

You could try to remove the cache of simple objects and see if that helps to reduce the frequency of the error

@andrejslogins
Copy link
Contributor Author

andrejslogins commented Feb 21, 2022

I updated the app with 5.2.1, and so far I stopped seeing these errors (added another dispose call for the canvas as well). I'll continue observing this, besides this one, I'm also getting error of TypeError: Cannot read properties of undefined (reading 'uniScaleKey'), though I'll create an issue for that later on, if needed
Likely will add a comment here after 24hrs or so, to have feedback in a longer span of time

@asturur
Copy link
Member

asturur commented Feb 26, 2022

i understand reproducing those bugs outside the context of your app is hard, but is also necessary to understand what is going on.
Did you try that idea of disabling the objectCaching for a test? Are you using picture filters by chance?

@stale
Copy link

stale bot commented Mar 27, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Issue marked as stale by the stale bot label Mar 27, 2022
@stale stale bot closed this as completed Apr 16, 2022
@ShaMan123
Copy link
Contributor

#7885 might solve this

@trevorweng
Copy link

trevorweng commented Jul 11, 2022

We're still seeing this as of fabric 5.2.1.

Our canvas code goes like this:

export function resetCanvas(canvas: Canvas) {
  $.contextMenu('destroy');

  try {
    canvas.dispose();
  } catch {
  }
}

const redrawCanvas = () => {
  if (canvasRef.current) {
    resetCanvas(canvasRef.current);
  }

  canvasRef.current = new fabric.Canvas('canvas', {
    height,
    selection: true,
    targetFindTolerance: 8,
    width,
});

It seems that contextTop is null during this call

if (this.upperCanvasEl) {
        this.__initRetinaScaling(scaleRatio, this.upperCanvasEl, this.contextTop);
      }

can we just add a simple check for that? or is this a symptom of something more troublesome?

@asturur @ShaMan123

@ShaMan123
Copy link
Contributor

ShaMan123 commented Jul 12, 2022

@trevorweng did you try the following?

#7885 might solve this


can we just add a simple check for that? or is this a symptom of something more troublesome?

something more troublesome for sure

@trevorweng
Copy link

@ShaMan123 will there be a new release with that fix? or should I patch-package our code?

@ShaMan123
Copy link
Contributor

Of course there will be a new release but you know how time frames are....
We are trying to push TS forward so we are prioritizing.
I suggest cloning the master branch and working on it directly, there's the v6-alpha branch as well or using patch-package.
You should decide how you want to act.
I would appreciate if you report to us if indeed the problem is fixed in the latest version or whatever you decide to use.

@CookedApps
Copy link

I am not sure if this is related to it, but we are seeing a weird Cannot read properties of null (reading 'clearRect') error on some of our clients and are not able to reproduce it. I followed the trace and seemingly contextContainer is null in some cases during first render. This also indicates a race condition, I think.

@ShaMan123
Copy link
Contributor

Yes it is related. The mentioned PR fixes the race condition

@CookedApps
Copy link

@ShaMan123 The PR fixes a race condition in the dispose function. Is dispose called on first render? Because we are seeing this error on first render. Not dispose. I am really not sure, sorry. 😄 Should I create a new issue?

@ShaMan123
Copy link
Contributor

It depends on your logic... I'm guessing you are using react.
The first thing you should do is trying out the fix.
After that open an issue

@trevorweng
Copy link

@ShaMan123 would it be possible to release a 5.2.2 with the dispose fix?

@ShaMan123
Copy link
Contributor

@trevorweng I am not in charge of releasing.

@ysyzqq
Copy link

ysyzqq commented Jan 30, 2024

@andrejslogins how to solve
TypeError: Cannot read properties of undefined (reading 'uniScaleKey')

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs_confirmation stale Issue marked as stale by the stale bot
Projects
None yet
Development

No branches or pull requests

6 participants