Model.onModelError doesn't allow dismissing the error and error message is always undefined

[don-sitebionics]

If you give sceneFilename a URL that is no longer available then the Model generates and error and I am able to catch that error in the onModelError event. While I am notified of the error there is no way to mark it is handled. The error makes it back to the page and the browser shows afull page unhandled exception error. Is there a feature that I'm missing, a needed feature, or should I be doing something on the page to catch this somehow? Note that model.errorMessage is also always null in the handler. I think the fix should be to pass another parameter allowing the handler to mark it as handled.

const ModelWithProgress: FC<ScaledModelWithProgressType> = (props) => {

  function onModelError(model: ILoadedModel): void {
    console.warn(model.errorMessage) // errorMessage is always undefined and no way to dismiss the error**
  }

  return (
    <SceneLoaderContextProvider>
      <Suspense
        fallback={
          <ProgressFallback
            progressBarColor={props.progressBarColor}
            rotation={props.progressRotation ?? props.modelRotation}
            center={props.center}
            scaleTo={1}
          />
        }
      >
        <Model
          name="model"
          reportProgress
          position={props.center}
          rootUrl={props.rootUrl}
          sceneFilename={props.sceneFilename}
          onModelError={onModelError}
          rotation={props.modelRotation}
          onModelLoaded={props.onModelLoaded}
        />
      </Suspense>
    </SceneLoaderContextProvider>
  )
}

[brianzinn]

Nice find - and I definitely need to make a change here. Sounds like you are thinking of something like stopPropagation()?
the code that throws the error is here:
https://github.com/brianzinn/react-babylonjs/blob/master/packages/react-babylonjs/src/hooks/loaders/useSceneLoader.tsx#L249

so, at that point if onModelError was provided, we could pass it an object like:

if (opts.onModelError) {
  opts.onModelError(loadedModel)
}
reject(exception ?? message)

What I have now doesn't actually make sense. I should be passing the message and the possibly undefined exception to the callback instead. In the end though I think I should still be rejecting. What about if you used an error boundary - I think that's how it's meant to be handled:
https://react.dev/reference/react/Component#catching-rendering-errors-with-an-error-boundary

[don-sitebionics]

Thanks for the reply!

Yes I was thinking of something like stopPropogation() or a return value from the onModelError event to tell if it was handled. And yes, passing the error vs the loadedModel to the onModelError makes more sense. ILoadedModel has an optional error message field but it doesn't get populated in the error case so it would be good populate it or to pass the error if adding a way to handle the error from the callback.

I'll check out using an error boundary. That was what I think I was missing as a way to work around this.

[don-sitebionics]

Unfortunately wrapping Model in an ErrorBoundary (or even further up the tree) did not work. Maybe has to do with how fiber bridges across?? I still get...
Uncaught runtime errors:
×
ERROR
Unable to load from https://...: The specified blob does not exist.
RuntimeError: Unable to load from https://...: The specified blob does not exist.
at errorHandler (http://localhost:3000/static/js/bundle.js:322596:38)
at errorCallback (http://localhost:3000/static/js/bundle.js:322461:9)
at http://localhost:3000/static/js/bundle.js:433684:5
at XMLHttpRequest.onReadyStateChange (http://localhost:3000/static/js/bundle.js:433812:13)

[brianzinn]

ok - i'll try to make a codesandbox to test it out...

[brianzinn]

hi @don-sitebionics
You are right - ErrorBoundary won't catch the async code throwing - I tried in a sandbox:
https://codesandbox.io/p/sandbox/codesandbox-react-tsx-forked-qzck7r
Why Error boundary doesn't work:
https://legacy.reactjs.org/docs/error-boundaries.html#how-about-event-handlers

Seems like an option might be to indicate to scene loader via opt-in if it should throw on error on simply return empty "model" object with an error attached.

I can also try/catch in the Model here (that might be confusing for developers as it's unexpected):
https://github.com/brianzinn/react-babylonjs/blob/master/packages/react-babylonjs/src/customComponents/Model.tsx#L56

What do you think? The error overlay if you had one was probably something for dev mode only (ie: the full page exception in browser), so maybe that's not a bad thing? What I have in sandbox above doesn't have an unhandled exception.

[don-sitebionics]

Sorry for the delay...

Model takes in an onModelError and an onModelLoaded. I think the right behavior is to call onModelLoaded when it succeeds and onModelError when it fails. But I think the signature for onModelError should be changed.

There is no reason to pass back a model when it hasn't been loaded.

Can...
onModelError?: (model: ILoadedModel) => void
be changed to something like...
onModelError?: (event: ISceneLoaderErrorEvent) => void

[brianzinn]

Ok. I’ll change the signature. That’s an easy fix - also, I won’t throw an error then.

[brianzinn]

I forgot about this - sorry. I am trying to clean up the outstanding issues. This one is relatively easy to fix - stay tuned...

[brianzinn]

@don-sitebionics I fixed the callback signature. I don't know if that will resolve as it is still throwing, so it may require an error boundary. I can not throw if that seems like a desired behaviour - Do you have time to try v3.2 and see if it works for you?

[don-sitebionics]

Hi Brian, Thanks for the update. I’m traveling right now but I’ll give a shot as soon as I get back. Many thanks! Don From: Brian Zinn ***@***.***> Sent: Monday, June 10, 2024 9:48 AM To: brianzinn/react-babylonjs ***@***.***> Cc: Don Gillett ***@***.***>; Mention ***@***.***> Subject: Re: [brianzinn/react-babylonjs] Model.onModelError doesn't allow dismissing the error and error message is always undefined (Issue #312) @don-sitebionics<https://github.com/don-sitebionics> I fixed the callback signature. I don't know if that will resolve as it is still throwing, so it may require an error boundary. I can not throw if that seems like a desired behaviour - Do you have time to try v3.2 and see if it works for you? — Reply to this email directly, view it on GitHub<#312 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/BCARIPYAE2MWSTJBLK3OYYLZGXKEJAVCNFSM6AAAAABFB2B426VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNJYHA2TKMBWGY>. You are receiving this because you were mentioned.Message ID: ***@***.******@***.***>>