Search code examples
reactjsreduxreact-redux

Handle unsuccessful Ajax calls with Redux and Redux Thunk in React


For my React components, I use Redux (react-redux) and Redux Thunk to maintain the app state.

There's a ShowGroup component that retrieves a group from the backend. If the group doesn't exist (or in any other error situation), an error callback is called to update the state.

class ShowGroup extends Component {
  constructor(props) {
    super(props);
    this.groupId = this.props.match.params.id;
    this.state = {
      'loaded': false,
      'error': null
    };
  }

  _fetchGroupErrorCallback = response => {
    this.setState({loaded: true, error: response});
  }

  componentDidMount() {
    this.props.fetchGroup(this.groupId, this._fetchGroupErrorCallback);
  }

  render() {
    //what is the best practice here ?
    let group = this.props.groups[this.groupId];
    if (!group) {
      if (!this.state.loaded) {
        return <div>Loading group ....</div>;
      }
      if (this.state.error) {
        return <div>{this.state.error.data.message}</div>;
      }
    }
    return (
      <div className="show-group">
        <form>
          {_.map(FIELDS, renderField.bind(this))}
        </form>
      </div>
    );
  }
}

function mapStateToProps(state) {
  return { groups: state.groups };
}

const mapDispatchToProps = (dispatch) => {
    return {
        fetchGroup: (id, callback) => dispatch(fetchGroup(id, callback)),
        updateGroup: (id) => dispatch(updateGroup(id))
    };
};

export default reduxForm({
  validate,
  //a unique id for this form
  form:'ShowGroup',
  fields: _.keys(FIELDS),
  fields_def: FIELDS
})(
  connect(mapStateToProps, mapDispatchToProps)(ShowGroup)
);

(it's a redux-form component that doesn't matter)

export function fetchGroup(id, fetchErrorCallback) {
  return (dispatch) => {
    axios.get(URL, AUTHORIZATION_HEADER)
      .then(response => {
        dispatch(groupFetched(response))
      })
      .catch(({response}) => {
        fetchErrorCallback(response);
        dispatch(groupFetchErrored(response));
      })
  };
}

groupFetchedErrored action creator:

export function groupFetchErrored(response) {
  //handle the error here
  return {
     type: FETCH_GROUP_ERROR,
     response
  }
}

And the reducer:

export default function(state=INITIAL_STATE, action) {
  switch(action.type) {
    //....
    case FETCH_GROUP_ERROR:
      return _.omit(state, action.response.data.id);
    default:
      return state;
  }
}

The problem:

A. In case of an error response, the component gets rendered three times: 1. First time it loads (which after the ajax call is made) 2. _fetchGroupErrorCallback is called and the state is set, which leads to a render 3. groupFetchErrored is dispatched which leads to another render

B. In case of successful response, the component gets rendered twice, BUT the state is not correct as nothing updates it (and I don't think it's correct to add a callback for it)

What is the best practice for handling ajax error responses when using Redux and Redux-Thunk with React ? How should the action creator and reducer notify the component that there's something wrong ?

Update 1

This is my root reducer:

const rootReducer = combineReducers({
  form: formReducer,
  groups: groupReducer
});

export default rootReducer;

As you can see, I have groups which is an object of form:

{
   groupId1: groupData1,
   groupId2, groupData2, 
   ....
}

So, another question is, how should I specify one group is loading (I cannot have isLoading or errorFetching on the groups level, and it doesn't seem to be a good idea to add one entry for each invalid group id user may try. Or maybe there's a way to put the invalid group id in the map and then clean it up ? I don't know where should that happen, though.


Solution

  • Put all state data related to the fetch into the Redux store. Then the store becomes your single source of truth.

    You can then omit fetchErrorCallback entirely.

    You already have a place in your reducer that's being notified of the error:

    case FETCH_GROUP_ERROR:
      return _.omit(state, action.response.data.id);
    

    You can store the error in the redux state right there:

    case FETCH_GROUP_ERROR:
      const stateWithoutId = _.omit(state, action.response.data.id);
      return {
        ...stateWithoutId,
        errorFetching: action.response.error,
        isLoading: false
      }
    

    Then pass errorFetching and isLoading in to your component via mapStateToProps like you're already doing with groups.

    Also to properly track isLoading it would be best to dispatch another action when you start the fetch (right before the call to axios.get), perhaps FETCH_GROUP_STARTED. Then in your reducer set it isLoading to true when that action type is dispatched.

    case GROUP_FETCHED: 
      return { 
        ...state, 
        isLoading: false,
        group: action.response.data // or however you're doing this now.
    
      };
    case FETCH_GROUP_STARTED:
      return { ...state, isLoading: true }; // or maybe isFetchingGroups is a better name
    

    If this were my code I'd make another change: Instead of passing response in to the action creators, I'd strip out just the relevant data and pass that to them. So the reducer doesn't need to know how to handle a response object; it just receives the data that is needed to display your component(s) correctly. That way all the code that handles axios calls and has to know about the shape of the objects they return is in one place - the action function. Something like:

    Action creators

    export function groupFetchErrored(errorWithId) {
      return {
         type: FETCH_GROUP_ERROR,
         payload: errorWithId
      }
    }
    
    export function groupFetchSucceeded(groupData) 
      return {
         type: GROUP_FETCHED,
         payload: groupData
      }
    }
    

    Action

    export function fetchGroup(id, fetchErrorCallback) {
      return (dispatch) => {
        axios.get(URL, AUTHORIZATION_HEADER)
          .then(response => {
            dispatch(groupFetched(response.data)) // or whatever chunk you need out of response
          })
          .catch(({response}) => {
            dispatch(groupFetchErrored({
                error: response.error, 
                id: response.data.id
              }
            );
          })
      };
    }
    

    Reducer

    You may have noticed I used action.payload in the action creators above.

    It's a common practice to always use the same name for the data property on dispatched actions (like always, in every reducer and every action creator across the entire app). That way your reducer doesn't need to be tightly coupled to the action creator; it knows the data is always going to be in the payload field. (Maybe that's what you were doing with response, but that seems like it's intended to be used specifically for http responses.)

    case GROUP_FETCHED: 
      return { 
        ...state, 
        isLoading: false,
        group: action.payload
      };
    case FETCH_GROUP_STARTED:
      return { ...state, isLoading: true }; // no payload needed
    case FETCH_GROUP_ERROR:
      const dataId = action.payload.id;
      const stateWithoutId = _.omit(state, dataId);
      return {
        ...stateWithoutId,
        errorFetching: action.payload.error,
        isLoading: false
      }