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

Proper way to handle errors / failure_actions #198

Open
donatoaz opened this issue Jul 10, 2018 · 2 comments
Open

Proper way to handle errors / failure_actions #198

donatoaz opened this issue Jul 10, 2018 · 2 comments

Comments

@donatoaz
Copy link

Hey everyone!

I started working on an app that uses redux-api-middleware but I am a little confused as to how error handling is/should be implemented.

Currently, the failure action is as such:

        ...
        {
          type: failure_action,
          payload: (action, state, res) => {
            if (errorInterceptor) return getJSON(res).then((data) => errorInterceptor(data, res))
            return getJSON(res).then((data) => {
              return {
                ...data,
                status: res.status
              }
            })
          },
          meta: (action, state, res) => {
            return {
              status: res.status,
              statusText: res.statusText
            }
          }
        }
        ...

In one of the containers there is a dispatch that sometimes gets a 404, and I would like it to act on that, and as such I did:

  dispatch(fetchWorkOrder(workOrderId)) // this might respond with a 404
    .then(
      () => { // this should only run if there is no error
        dispatch(loadServicingsForWorkOrder(workOrderId, 'unfinished'))
        dispatch(fetchUnreadMessagesForWorkOrder(workOrderId))
      },
      (e) => {
        console.log('This is not running...', e)
      }
    )
    .catch((e) => console.log('And neither is this...', e))

I tried the following: passing an errorInterceptor for the payload on failure_action:

    errorInterceptor: (data, response) => {
      console.log('getting error here')
      Promise.reject(new ApiError('testing reject'))
    }

But that did not give me the desired results.

I ended up changing the Promise.reject with a throw new ApiError(response.status, response.statusText, response) and on my container file, I changed the dispath logic above to:

  dispatch(fetchWorkOrder(workOrderId)) // this might respond with a 404
    .then(
      ({meta : {status = null} = {}}) => {
        if (status === 404) {
          dispatch(push('/work-orders'))
        } else {
          dispatch(loadServicingsForWorkOrder(workOrderId, 'unfinished'))
          dispatch(fetchUnreadMessagesForWorkOrder(workOrderId))
        }
      })

But it seems like the wrong thing to do, specially because there may be other errors different than 404...

What is the suggested practice in these cases?

@nason
Copy link
Collaborator

nason commented Jul 11, 2018

Hi @donatoaz, great question!

I tried the following: passing an errorInterceptor for the payload on failure_action

I'm not sure what errorInterceptor is -- that's not part of this lib. How should that work?

In one of the containers there is a dispatch that sometimes gets a 404, and I would like it to act on that

So I'm understanding that you want 404s to be treated as FAILURE types (default behavior), but with some special logic. Is that right?

If so, would something like the following work?

// Part of the RSAA action
{
  type: failure_action,
  payload: (action, state, res) => {
    return getJSON(res).then((data) => {
      return {
        ...data,
        status: res.status
      }
    })
  }
}
dispatch(fetchWorkOrder(workOrderId)) // this might respond with a 404
  .then(
    (action) => {
      if (action.error) {
        if (action.payload.status === 404) {
          dispatch(push('/work-orders'))
        } else {
          // Other errors
        }
      } else {
        // this should only run if there is no error
        dispatch(loadServicingsForWorkOrder(workOrderId, 'unfinished'))
        dispatch(fetchUnreadMessagesForWorkOrder(workOrderId))
      }
    }
  )

I do not think there is any case in which redux-api-middleware rejects the promise returned to dispatch, which is why I think your example wasn't working (it's using the onRejected callback to then)

https://github.com/agraboso/redux-api-middleware#error goes into some detail about this -- please let us know if this isn't enough, or if there's something missing that you think would be helpful.

@NipperInShorts
Copy link

I hope no one minds I piggyback on this, it seems sort of inline with what I'm struggling with. Also, this is the first time I've posted an issue to GitHub so feedback on how to improve communicating in this type of forum is greatly appreciated.

Currently, I have a function that will send a REQUEST FSA, the server responds with a 400 and the REQUEST never seems to resolve and move into the FAILURE FSA unless I add a descriptor. My limited knowledge of how this works makes me believe that a FAILURE should run.

This code doesn't call a FAILURE FSA after receiving a 400 like I would assume it would.

export const submitCartOrder = (params: CartParams): Thunk => {
  return async (dispatch: Dispatch) => {
    const result = await dispatch(
      apiAction({
        endpoint: "cart/order/submit",
        method: "POST",
        body: JSON.stringify(params),
        types: [
          SubmitCartOrderActions.SUBMIT_CART_ORDER_REQUEST,
          SubmitCartOrderActions.SUBMIT_CART_ORDER_SUCCESS,
          SubmitCartOrderActions.SUBMIT_CART_ORDER_FAILURE,
        ],
      }),
    );
    if (result.type === SubmitCartOrderActions.SUBMIT_CART_ORDER_FAILURE) {
      throw new Error(result.payload);
    }
    return result;
  };
};

However, this will once the server responds.

export const submitCartOrder = (params: CartParams): Thunk => {
  return async (dispatch: Dispatch) => {
    const result = await dispatch(
      apiAction({
        endpoint: "cart/order/submit",
        method: "POST",
        body: JSON.stringify(params),
        types: [
          SubmitCartOrderActions.SUBMIT_CART_ORDER_REQUEST,
          SubmitCartOrderActions.SUBMIT_CART_ORDER_SUCCESS,
          {
            type: SubmitCartOrderActions.SUBMIT_CART_ORDER_FAILURE,
            // promise would not resolve from request when 400.
            // unless using a descriptor with a payload.... no idea
            payload: async (action: any, state: any, res: Response) => {
              const payload = await res.json();
              return payload;
            },
          },
        ],
      }),
    );
    if (result.type === SubmitCartOrderActions.SUBMIT_CART_ORDER_FAILURE) {
      throw new Error(result.payload);
    }
    return result;
  };
};

For some context, the apiAction({...}) takes the data we want to pass to our middleware to generate sessions, headers, and the like and then passes that to redux-api-middleware.

Some potentially important lib versions:

    "react-redux": "^6.0.0",
    "redux": "^4.0.1",
    "redux-actions": "^2.3.0",
    "redux-api-middleware": "^2.3.0", // i know this is an older version, we've branched and upgraded - same issue... btw a second request never fires like in some instances where it sets error to true
    "redux-persist": "^5.10.0",
    "redux-thunk": "^2.2.0",

Does this make sense? I'd be happy to provide any more context needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants