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

RTK Query gets stuck pending and swallows errors thrown in addMatcher reducers #3795

Open
adamerose opened this issue Oct 11, 2023 · 21 comments
Assignees
Labels
bug Something isn't working rtk-query

Comments

@adamerose
Copy link

adamerose commented Oct 11, 2023

I just ran into this problem and found this comment saying it's expected behavior, but I'm making this issue with a clearer example for further discussion because this seems like a footgun that should be changed.

Below is an example with a login button. Click the button and it sends a POST request to the backend with credentials to log in and gets back user info + token, and stores them in the authSlice and displays the username.

https://codesandbox.io/p/github/adamerose/rtkq-reducer-error-example/master
https://github.com/adamerose/rtkq-reducer-error-example

With the error thrown in the matchFulfilled reducer, the query gets stuck forever in this state even though the HTTP request completed successfully:

isUninitialized:false
isFetching:true
isSuccess:false
isError:false
const authSlice = createSlice({
  name: "auth",
  initialState: initialState,
  reducers: {
    logout: (state) => {
      state.user = null
    },
  },
  extraReducers: (builder) => {
    builder.addMatcher(
      api.endpoints.login.matchFulfilled,
      (state, { payload }) => {
        state.user = payload.user

        throw Error("Example error")
      },
    )
  },
})

It also seems to completely swallow the error and nothing appears in the console, which made it very hard to figure out why I was getting stuck pending in an actual project. I've included a separate Throw Error button that throws an error in a normal reducer and I can see it appear in the console, but for some reason the one inside the addMatcher example doesn't. Am I doing something wrong here?

image image image
@markerikson
Copy link
Collaborator

At first glance, that sounds like normal Redux behavior.

If you look at the Redux core inside of dispatch(), it does:

    try {
      isDispatching = true
      currentState = currentReducer(currentState, action)
    } finally {
      isDispatching = false
    }

So any error in a reducer gets swallowed.

Generally speaking, reducers are supposed to be pure functions, and philosophically that means they shouldn't throw errors at all.

I get the point that you're making, and agree it would be conceptually nice to have a better indicator here. But there's only one root reducer function, which is called to return a new state, and if any code inside that reducer errors there isn't a state update applied at all.

How and why are you running into cases where a reducer is throwing an error?

@adamerose
Copy link
Author

Generally speaking, reducers are supposed to be pure functions, and philosophically that means they shouldn't throw errors at all.

I'm only explicitly throwing an error in the code here as a minimal reproducible example. It's not like we can simply decide to never have any errors occur in a reducer right? Errors are unavoidable and not all errors are caused by side effects, maybe I just have a typo or try to access an undefined property.

How and why are you running into cases where a reducer is throwing an error?

Here is the code that initially caused this for me:

  extraReducers: (builder) => {
    builder.addMatcher(api.endpoints.login.matchFulfilled, (state, { payload }) => {
      state.token = payload.token;
      const decodedToken: AuthJwtToken = jwtDecode(payload.token);
      console.log(decodedToken, payload.token);
      state.username = decodedToken.username;
    });
  },

Not sure what the error was because it got swallowed...

So any error in a reducer gets swallowed.

Doesn't my Throw Error button demonstrate that isn't true for regular reducers?

image

Anyways, swallowing errors anywhere seems like a problem.

@markerikson
Copy link
Collaborator

Can you record a Replay ( https://replay.io/record-bugs ) of the original issue, and share that here? That would make it a lot easier for me to see what's going on.

@adamerose
Copy link
Author

Unfortunately it's a company project so I can't share a replay - but I think my codesandbox or repo demonstrate the issue. Any errors inside a reducer in builder.addMatcher for matchFulfilled of an RTKQ query get swallowed and end up in with RTKQ having incorrect pending state.

You could replace throw Error("Example error in addMatcher") here with a typo like state.user = paylaod.user or a function call to an external library like lodash or anything else that might throw an error. Surely I can't be the first person trying to debug an error inside a reducer?

@EskiMojo14
Copy link
Collaborator

Generally quite confused by this discussion.

At first glance, that sounds like normal Redux behavior.

If you look at the Redux core inside of dispatch(), it does:

try {
  isDispatching = true
  currentState = currentReducer(currentState, action)
} finally {
  isDispatching = false
}

So any error in a reducer gets swallowed.

I don't see anywhere an error would be swallowed here? Errors thrown in a reducer will be uncaught as usual, as shown by the "Throw error" in the linked CSB. Though as covered, reducers should never throw unless something is very wrong.

Errors thrown in an addMatcher reducer shouldn't be caught either: Error reducer playground

Whatever's happening with regards to the error being swallowed seems very specific to RTKQ - either during its middleware, thunks, or hooks.

@markerikson
Copy link
Collaborator

@markerikson
Copy link
Collaborator

Weird. It looks like the error goes all the way back up to the stack... and then gets swallowed by an async-to-generator-transpiled body in redux-toolkit.esm.js?

@markerikson
Copy link
Collaborator

And if I re-record the example with 2.0-beta.2 (which uses real async), it seems like we just unwind the stack out to createAsyncThunk and it vanishes:

https://app.replay.io/recording/rtk-3795-error-in-a-reducer-rtk-20-beta2--35fad959-c1d4-4ce2-8365-dabf3340ee6e

I'm actually pretty confused, tbh. It seems like the error does get thrown upwards, and then just vanishes.

Unfortunately I'm busy enough that I don't have time to look into this further. If you or anyone else does and can figure out what's going on, I'd be curious to know details.

@adamerose
Copy link
Author

I couldn't find it in your replay but I stepped through in my debugger locally and I'm pretty sure the error is getting swallowed here:

.catch((error) => ({ error }))

I commented that line out in node_modules and began seeing the error in my console. However this still doesn't fix RTKQ getting stuck pending.

image

@markerikson
Copy link
Collaborator

Ah, interesting. I assume that's to avoid repetitive console logs about uncaught errors.

Like I said, nothing can fix the "stuck pending" problem. The reducer was supposed to update the state, but there was an error in the reducer, so no state updates got saved for that action. That's architecturally defined - state = reducer(state, action) failed.

The answer to that truly is "don't have errors in a reducer".

@adamerose
Copy link
Author

adamerose commented Oct 12, 2023

The reducer was supposed to update the state, but there was an error in the reducer, so no state updates got saved for that action. That's architecturally defined - state = reducer(state, action) failed.

Can you expand on this?

  1. What should my Redux state be after a reducer throws an error? Do we expect it to stay as it was before the reducer ran, or is it undefined (ie. depending on the error, our state could be anything and the app has to be hard restarted)?
  2. Should the action that triggered the failing reducer still show up in dev tools?
  3. Should the other reducers triggered by that same action all automatically fail too? It seems like this is the cause of RTKQ getting stuck pending here.
  4. Should there be some indication in the dev tools that a reducer/action failed with an error?

The answer to that truly is "don't have errors in a reducer".

That is always the goal, but if a reducer error does happen in production having things fail non-catastrophically would be ideal.

In the case of my original demo - updating the RTKQ query statusFlags isn't logically tied to my custom reducer for storing the auth response, they are just two separate things that need to happen when my HTTP request succeeds, so I would want my reducer failing to mean auth.user fails to update but not to also break RTKQ and have it get stuck as pending. What do you think of this as a potential solution?

  • Have separate actions (eg. internalFulfilled) for internal RTKQ functionality like updating statusFlags, and don't expose these to the user through the RTK API docs
  • Both fulfilled and internalFulfilled actions would be dispatched upon a successful query
  • Have the existing matchFulfilled matcher connect only to fulfilled for users like me to use in user-land
  • Internal actions could be hidden in the dev tools by default, similar to how you already put RTKQ actions in a tab separate from user actions. And I doubt the two actions would interfere or have race conditions since one only modifies state internal to RTK like statusFlags that users should never be touching with our own reducers.

Or an alternative solution would be to make it so that if one reducer throws an error, the other reducers for the same action still complete. I'm not sure if this goes against fundamental Redux architecture.

@markerikson
Copy link
Collaborator

What should my Redux state be after a reducer throws an error? Do we expect it to stay as it was before the reducer ran

Yes, exactly that. The reducer function threw, no new state was returned, therefore the store's currentState variable was not updated.

Should the action that triggered the failing reducer still show up in dev tools

Not 100% sure. The devtools piece is an enhancer that wraps around the original store.dispatch(). I assume that it only sends the action to the extension after the real store.dispatch() is complete, so that it can get the current state and serialize it. So, there also, if an error is thrown in the reducer, that enhancer code to get the state never runs.

Should the other reducers triggered by that same action all automatically fail too?

Yes, because there is one root reducer function, and all of the slice reducers are actually part of that one root reducer function.

Should there be some indication in the dev tools that a reducer/action failed with an error?

The devtools UI has no provision for that that I know of, and that ties into point 2.

updating the RTKQ query statusFlags isn't logically tied to my custom reducer for storing the auth response, they are just two separate things that need to happen when my HTTP request succeeds

But that's part of the point here.

The fulfilled action only got dispatched once. RTKQ depends on that.

You've added extra reducer code, that runs in the same root reducer, handling the same action. So when the one call to rootReducer(state, rtkqFulfilledAction) errors, that prevents the RTKQ fulfillment update from being saved too.

I get that this is a pain point. But it ultimately comes down to "when someFunction() throws an error, it doesn't return anything" as a basic piece of JS execution behavior, and in this case the function is actually rootReducer().

We aren't going to rearchitect RTKQ's internals just to try to work around the chance that a user might add extra reducers listening to the same action, and those reducers might someday throw an error. We have to write the code assuming the RTKQ reducer logic is the only code that cares about the action, and that our logic handles it correctly.

@adamerose
Copy link
Author

You've added extra reducer code, that runs in the same root reducer, handling the same action. So when the one call to rootReducer(state, rtkqFulfilledAction) errors, that prevents the RTKQ fulfillment update from being saved too.

Okay that makes sense, so it sounds like the only fix would be separate actions like I described. Or in my own code I can wrap all reducers that touch internal RTK actions with a try/catch that returns initial state on failure.

We aren't going to rearchitect RTKQ's internals just to try to work around the chance that a user might add extra reducers listening to the same action, and those reducers might someday throw an error.

That's fair enough. Maybe reconsider it if you come across other problems caused by mixing internal and user actions, I agree this issue alone wouldn't justify the effort. If the error swallowing gets fixed then this becomes a much smaller pain point.

Thanks for all your work, and quick response time!

@markerikson
Copy link
Collaborator

Btw, not sure I actually got this detail: what was the actual error that was happening in your real code to cause this originally?

@adamerose
Copy link
Author

In the code I pasted earlier:

  extraReducers: (builder) => {
    builder.addMatcher(api.endpoints.login.matchFulfilled, (state, { payload }) => {
      state.token = payload.token;
      const decodedToken: AuthJwtToken = jwtDecode(payload.token);
      state.username = decodedToken.username;
    });
  },

I had a mismatch in the types defined for my backend and frontend. My backend responded with an object with the access_token field but my frontend expected it in the token field. So despite having types in my api endpoint generics and TypeScript having no complaints, payload.token was undefined and calling jwtDecode(undefined) threw the error Invalid token specified.

@markerikson markerikson closed this as not planned Won't fix, can't repro, duplicate, stale Dec 6, 2023
@adamerose
Copy link
Author

What's the reason this was closed?

To recap the thread...

  • When an error is thrown inside a reducer attached to matchFulfilled, RTK gets permanently stuck pending and the error is not logged anywhere (not in dev tools nor console).
  • Reducers should ideally never throw errors, so this problem of getting stuck pending is a wont fix and expecting RTK to gracefully recover from reducer errors is out of scope.
  • However the swallowing of these errors and having them not appear in the console is a bug that still needs to be fixed, right?

@phryneas
Copy link
Member

phryneas commented Dec 6, 2023

We're a bit at the mercy of how JavaScript handles errors here - every kind of error in an async context will throw in the same way through the same problem - we can't really distinguish between errors from different sources, and generally, errors should just end up in the store, but not bubble globally, or people would get "uncaught exception" warnings for expected errors.

The problem here is that this occurs in the exact spot where that error would also be written, so I don't really know what to do about it. I'll reopen this (it was one of many many issues that were closed as 2.0 was released), but I don't know a good solution.

@phryneas phryneas reopened this Dec 6, 2023
@markerikson
Copy link
Collaborator

I was doing a cleanup sweep through our issues list and trying to close anything that looked resolved or unactionable.

Yeah, as we've discussed, there's nothing that can be done on the reducer side - that's inherent to how Redux works.

I'm not clear if there's anything we can do internally beyond that.

@adamerose
Copy link
Author

errors should just end up in the store, but not bubble globally, or people would get "uncaught exception" warnings for expected errors.

Doesn't the codesandbox demo in my original question show that RTK normally does bubble errors up to the console where the user can see them? My issue is just about errors being swallowed in reducers specifically attached to internal RTK actions. I'm confused how this is a problem inherent to how Redux works, or why it would be unsolvable.

image

Can anyone comment on the snippet I mentioned here?
#3795 (comment)
This seems to just silently swallow the error, or is that not what's going on here?

@markerikson markerikson self-assigned this Feb 5, 2024
@markerikson markerikson added bug Something isn't working rtk-query labels Feb 5, 2024 — with Volta.net
@AndreaDigiTrails
Copy link

This seems somewhat related to the issue that I have with a Next.js 14 application where I used RTK Query to handle the authentication phase of the application.

I still have to figure out where the main issue is but the thing is that I built a Context that wraps my layout.tsx. Inside it I use RTK Query to ping an endpoint (user/me) just to check if the user is authenticated.

On many of the routes this works just fine, if my BE respond with a 401, thanks to the re-authorization tutorial on RTK Query website I am able to perform a refresh API call but there is a specific URL/button that sometime just fails to work.

The URL I am trying to see is the default page that user sees after a successful login, see it as a 'Back home button', and it happen that when I try to see that route by clicking on a Link component it does not call the user/me endpoint but looks like Next.js tryes to load a cached version of the page.

My issue is that I do not see any error or enpoint pinged and it really confuse me.

Since I am not able to figure out what the issue is I am in the middle of a refactor aimed at removing the Redux/RTK Query dependency and handle it with a combination of server and client components with Next.js

I just wrote this comment in the hope that someone have faced something similar and is able to help me out.

If you need more details on how the application works I am adding them below but unfortunately I am not able to build a full example.

Little ReAuth difference

My fetchBaseQuery uses the credentials: 'include' as my BE is able to generate httpOnly cookies.

The Auth Context

This is the 'Context' that I use in order to handle the loading state of the application, and it is the one where I notice that's stuck on 'Loading...' when I try to check the URL mentioned above.

"use client";

import { useEffect } from "react";
import { redirect } from "next/navigation";
import { useAppSelector, useAppDispatch } from "@/redux/hooks";
import { setAuth, finishInitialLoad } from "@/redux/features/authSlice";
import { useUserQuery } from "@/redux/features/authApiSlice";

export default function RequireAuth({
  children,
}: {
  children: React.ReactNode;
}) {
  const { isAuthenticated, isLoading } = useAppSelector((state) => state.auth);
  const dispatch = useAppDispatch();
  const { status } = useUserQuery();

  useEffect(() => {
    if (status === "rejected") {
      redirect("/");
    }

    if (status === "fulfilled") {
      dispatch(setAuth());
      dispatch(finishInitialLoad());
    }
  }, [status, dispatch]);

  if (isLoading) {
    return <div className="flex justify-center my-8">Loading...</div>;
  }

  if (!isAuthenticated) {
    redirect("/");
  }

  return <>{children}</>;
}

@markerikson
Copy link
Collaborator

@AndreaDigiTrails based on your description I'm not sure you're seeing the same problem described in this issue.

That snippet isn't enough to actually give me any sense of what might be happening. Any chance you can share a CodeSandbox, Github repo, or Replay ( https://replay.io/record-bugs ) of the problem?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working rtk-query
Projects
None yet
Development

No branches or pull requests

5 participants