SolidJS•2y ago

Effects not running after a unhandled promise rejection event

When an unhandled error event occurs (e.g. by throwing new Error()), the ErrorBoundary is catching the error. When an unhandled promise rejection occurs, the UI/JSX is being updated, I display the state of the resource in the JSX, and it switches from pending to errored, but the createEffect functions are not running:

createEffect(() => {
  console.log('userResource.state', userResource.state);
});

createEffect(() => {
  console.log('userResource.state', userResource.state);
});

When displaying {userResource.state} from the JSX, it prints pending and then updates to errored because there was a promise rejection. It reacts. However the createEffect above only logs pending once and will not log errored, while the JSX did react/get updated.

const EnsureAuthenticated: VoidComponent = () => {
  const [currentUser] = useCurrentUser();

  createEffect(() => {
    // doesn't react (logs only pending once)
    console.log('currentUser.state', currentUser.state);
  });

  return (
    // reacts (displays pending then errored)
    <>{currentUser.state}</>
  );
};

const EnsureAuthenticated: VoidComponent = () => {
  const [currentUser] = useCurrentUser();

  createEffect(() => {
    // doesn't react (logs only pending once)
    console.log('currentUser.state', currentUser.state);
  });

  return (
    // reacts (displays pending then errored)
    <>{currentUser.state}</>
  );
};

26 Replies

Mathieu•2y ago

The problem is that I throw in some createEffect, which seems to break the subsequent createEffects. Commented on the issue https://github.com/solidjs/solid/issues/1744#issuecomment-1576305964

high1•2y ago

Throwing errors in effects certainly doesn't look like a good idea to me I see what's the goal - to have the error bubble up to the global listeners, but that's not the way to do it. There are two options here - either don't use createResource, or expose the global event listener functions, and call them inside the effect as a workaround. Or use the ErrorBoundary...

Mathieu•2y ago

I could indeed reuse some function. Now I call setTimeout(() => { throw e }, 0) which has been working fine to make the error bubble up.

high1•2y ago

You are opting out of the tracking system with that setTimeout I would strongly advise that you rethink the strategy of bubbling exceptions to the top just to log them.

Mathieu•2y ago

yes but I don't need the tracking system, I want to throw outside of the synchronous execution (so that asynchronously the global listener sends the error to some reporting tool). It has been working fine consistently. Would you mind sharing me why that could be considered bad code for this case?

high1•2y ago

It bubbles up through all the layers of your code, and a lot of them will break on an unexpected error. I would assume that React and many others will do the same - the rendering system would just crash.

Mathieu•2y ago

with the setTimeout? Don't I get outside of the synchronous execution and so, not interfering with Solid "work"?

high1•2y ago

You are opting out of the system

Mathieu•2y ago

ah but wait, I still handle the error in addition to making the error bubbling up

high1•2y ago

That's even worse... Rethrowing the error just for the sake of logging I would advise a different approach for that - since the error is already handled, just log the exception in a different way - you have context in solid, or you could export global logging functions.

Mathieu•2y ago

yeah I could simply use some global function to log the error. I think your argument if I get you right, is that the rethrow with setTimeout is a little hacky. I don't think it interferes with Solid's execution so it will consistently reach the global listener without interfering with the reactive system, however it's too hacky? I got your thoughts right?

high1•2y ago

It's not too hacky, it's just .... inefficient. It will work in this case, however, it will require circumventing each and every one of the code layers which do not like unhandled exceptions - which I assume are a majority. I would rather just handle the error where it occurs and use a global logger

Mathieu•2y ago

ok there's just one core thing I don't get, is how some code layer could intercept an unhandled exception thrown from a settimeout

high1•2y ago

It could not But you are scheduling a microtask, and rethrowing an exception to probably just log an error And I don't like that at all

Mathieu•2y ago

haha I see, the global logger is a nice idea anyways, thank you so much for the help as always!<3

high1•2y ago

The global logger is something that permiates through all of your code. Not sure if it's such a nice idea But there are ways to implement it to not require bubbling up of exceptions And I suggest those However, to each his own

Mathieu•2y ago

learned a new word too:D but apparently spelled permeates

high1•2y ago

Not a native speaker, so forgive my english

Mathieu•2y ago

same

high1•2y ago

In the class based world, you would either have to have a logger as a dependency injected in the constructor to be clean

Mathieu•2y ago

generally speaking in JavaScript, what would be the disadvantage of that in your opinion? Because with the IDE search functions you can find calls easily. As long as it is used in application code (not some generic system where we'd rely more on dependency injection), I can't really think of a disadvantage. And I can imagine a lot devs importing global functions like utility functions throughout the app. It's some form of bad inheritance it's true. but I can't find the arguments right now against it

high1•2y ago

It's inefficient It's a global logger implementation attached to the window Which requires the exception to bubble up Why don't you use a singleton and log the error where it occurs? the logger is just an interface

Mathieu•2y ago

I'm talking about the solution of exporting a global logger function and what you said:

The global logger is something that permiates through all of your code. Not sure if it's such a nice idea

In my code I'd have:

window.addEventListener('unhandledrejection', handleError);
window.addEventListener('error', handleError);

window.addEventListener('unhandledrejection', handleError);
window.addEventListener('error', handleError);

and I could export this handleError and import that function instead of rethrowing

high1•2y ago

True You can also put those in a context But I like plain export more

Mathieu•2y ago

but it's funny that on the server I'd be shocked if I see something like this. We'd go for dependency injection.

high1•2y ago

these two should be there just a last safety line And then all your classes would depend on a logger On which they do not really depend at all

Gaming

Programming

Effects not running after a unhandled promise rejection event