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

[Recipe] API error handling #74

Open
patrickgordon opened this issue May 6, 2016 · 11 comments
Open

[Recipe] API error handling #74

patrickgordon opened this issue May 6, 2016 · 11 comments
Labels

Comments

@patrickgordon
Copy link

patrickgordon commented May 6, 2016

I was struggling to come up with an effective way to leverage the failure FSAs that are dispatched by this package to handle API errors (mainly 401 and 403 responses) in a DRY manner. I have come up with this approach and thought it might be useful for some others and, if possible, for comment from the package contributors if I'm missing something...

General concepts:

  • A new middleware to handle ApiErrors
  • The Failure FSA needs to be customised to include some information in the meta
  • A function which acts as a handler and passes to the appropriate action
  • An action to handle the failure

Here's an example:

Middleware:

// apiError.js
import {logout} from '../modules/auth'

export default store => next => action => {

  // Checks to see if the action has a payload and if the payload is an ApiError

  if (action.payload && action.payload.constructor.name === 'ApiError') {
    if (action.payload.status === 403) {
      if (action.error && action.meta) {
        store.dispatch(action.meta.handler(action.meta.pushTo, action.meta.errorMsg))
      } else {
        // To avoid getting stuck in middleware.
        return next(action)
      }
    } else if (action.payload.status === 401) {
      var errorMsg = 'Your session has expired. Please login again.'
      store.dispatch(logout(true, errorMsg))
    }
  } else {
    // So the middleware doesn't get applied to every single action
    return next(action)
  }
}

Redux Module (as per react-redux-starter-kit) which has actions, constants, and reducer:

// contacts.js - my redux module
...

//Action to call update with extended description 
export function updateContact(selfLink, params) {
  return {
    [CALL_API]: {
      method: 'put',
      headers: {
        'Accept': 'application/json',
        'Content-Type': 'application/json',
        'Authorization': 'Bearer ' + localStorage.getItem('id_token')
      },
      endpoint: BASE_API_URL + selfLink,
      types: [
        UPDATE_CONTACT_REQUEST,
        UPDATE_CONTACT_SUCCESS,
        {
          type: UPDATE_CONTACT_FAILURE,
          meta: {
            handler: handleContactError,
            pushTo: '/contacts',
            errorMsg: 'Sorry, you\'re not able to update this contact'
          }
        }
      ],
      body: params
    }
  }
}

/**
 * This function is the handler for any contact error. It will push to the provided route (pushTo) and then dispatch an
 * action with the error message to display. The reducer consequently will set the isError state to True which can be
 * used to display in the view.
 * @param pushTo {String} - a string which represents a route - e.g. '/' or '/contacts'. Defaults to '/'
 * @param errorMsg - a string which can be displayed to the end-user
 * @returns {function()}
 */
export function handleContactError(pushTo = '/', errorMsg = null) {
  return dispatch => {
    dispatch(push(pushTo))
    dispatch(contactError(errorMsg))
  }
}

/**
 * This action is called when both the isError flag and errorMsg params are present
 * @param errorMsg {String} - The error message which has caused the logout
 */
function contactError(errorMsg) {
  return {
    type: CONTACT_ERROR,
    payload: {
      errorMsg: errorMsg
    }
  }
}

...

// IN THE REDUCER
    case CONTACT_ERROR:
      return Object.assign({}, state, {
        isFetching: false,
        isError: true,
        errorMsg: action.payload.errorMsg
      })

The general idea being that if you extend to have more modules - that the same concepts as above can be done and the middleware will handle it if there is a failure. This is also true of other actions in the same module. For example, I can also extend the failure for the fetchContact(id) action and if it fails due to a 403 then this will be handled by the middleware as well.

I hope this is useful / the appropriate place to put this. Would love to hear your thoughts on how this could be improved further.

@mlippens
Copy link

mlippens commented May 9, 2016

Hey @patrickgordon this is a good idea and also more or less how I implemented it. I especially like how you've used meta to be able to push the handler from within your action.

To give an alternative to this approach: I just push a redirect in my middleware, and then I catch the ApiErrors again in the reducers where I want to process them. Your approach however does abstract this away in your handler along with the redirect info: (i.e. you only need to deal with ApiError only once)

The only thing I would make better is your checking of the meta object you are passing, as that it provides better error messages when something unexpected happens when programming. As such, you could assert that typeof handler === "function", and that your pushTo and errorMessage also are conform with what you expect.

@patrickgordon
Copy link
Author

Thanks @froginvasion for the feedback. That's a good idea to check that what is in meta is the right type.

I have also now extended the middleware to handle 404s as well which pushes to a NotFound component at the /404 route.

@nvartolomei
Copy link

@froginvasion

I just push a redirect in my middleware, and then I catch the ApiErrors again in the reducers where I want to process them.

can you be a little bit more explicit here? a code example maybe

@mlippens
Copy link

mlippens commented May 10, 2016

Well it's just a simpler version of @patrickgordon who provided extra things to better handle his API failures. I just do this:

//isUnauthorized checks if it is an instance of ApiError and checks status codes
let middleware = (store) => (next) => (action) => {
    if (isUnauthorized(action)) {
        store.dispatch(push('/login'));
    }
    next(action);
};

I make sure the original action is just passed on to the reducers no matter what. I don't transform it here, but rather leave it up to the reducers what they want to do with it. Then I have some reducers (those that hold errors) that will take that original action and store it if needed. But for me, I don't really need the extra functionality to be able to push them to different locations based on the action, making my middleware simpler (but also much more implicit).

Then in a reducer I can reuse the same function that checks if the action is an ApiError:

export default function(state = Map(), action) {
  if (isUnauthorized(action)) {
    return state.merge({message: "Your session has been expired!", className: 'danger'});
  }
}

Transforming it in something else has the advantage however that you only need to deal with ApiErrors from redux-api-middleware in one single place, which makes it more DRY than my approach. Whenever you would need to deal with another status code or error, you could just do that right there in your own middleware.

Originally, I thought wow this is ugly, but the more I think about it the more logical this place seems to me.

@mlippens
Copy link

@patrickgordon can you update your example with the 404 action you dispatch? I'm interested to see how you did it ;)

@patrickgordon
Copy link
Author

@froginvasion sure - here's the updated apiError.js with the 404 handling added in.

// apiError.js

import {logout} from '../modules/auth'
import {push} from 'react-router-redux'

export default store => next => action => {

  // Checks to see if the action has a payload and if the payload is an ApiError

  if (action.payload && action.payload.constructor.name === 'ApiError') {
    console.log(action)
    if (action.payload.status === 403) {
      if (action.error && action.meta) {
        store.dispatch(action.meta.handler(action.meta.pushTo, action.meta.errorMsg))
      } else {
        // To avoid getting stuck in middleware.
        return next(action)
      }
    } else if (action.payload.status === 401) {
      var errorMsg = 'Your session has expired. Please login again.'
      store.dispatch(logout(true, errorMsg))
    } else if (action.payload.status === 404) {
      store.dispatch(push('/404'))
    }
  } else {
    // So the middleware doesn't get applied to every single action
    return next(action)
  }
}

Nothing too special happening here - thinking about ways I might extend that to allow for custom messages and routes to be handed to the NotFound component.

@joeyfigaro
Copy link

@patrickgordon Can you post your logout module?

@patrickgordon
Copy link
Author

As in to logout from my Web app?

I just destroy the JWT token from storage, redirect to homepage, and reset
store.

On Wed, 12 Oct 2016, 4:43 PM Joey Figaro [email protected] wrote:

@patrickgordon https://github.com/patrickgordon Can you post your
logout module?


You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#74 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ADCPwmGpp-Rb-bZbiABp6Br0BZMLvZSqks5qzOxTgaJpZM4IYi0a
.

@eladlevy
Copy link
Contributor

eladlevy commented Jan 1, 2017

I really like the solution but I would avoid using action.payload.constructor.name for checking the payload's type since the constructor name might get changed upon minification. If your'e using webpack see: this thread

Instead I simply check action.payload.name === 'ApiError'

@patrickgordon
Copy link
Author

Hey @eladlevy - yep that's something I ran into in production which was doing my head in until I figured it out. I have actually changed it to exactly what you have said:
action.payload.name === 'ApiError'

I should probably update this with the latest 'solution'.

I was actually thinking of packaging this up as its own npm lib too

Cheers.

@tylercrosse
Copy link

tylercrosse commented Mar 23, 2017

How do you handle/catch the 404 Error that isomorphic fetch throws? It would be nice to at least keep it out the console.

@nason nason added the docs label Nov 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants