Uncaught (in Promise) issue

[hotay]

When the API return an error, return Promise.reject(newAction); this line throw an uncaught exception. Which need to be caught like this store.dispatch(someAsyncAction).catch(err => err) otherwise it will throw an Uncaught (in Promise) error. I think return newAction is good enough there.

return client.request(options.getRequestConfig(action))
      .then(
        (response) => {
          const newAction = options.onSuccess({ action, next, response, getState, dispatch }, options);
          options.onComplete({ action: newAction, next, getState, dispatch }, options);

          return newAction;
        },
        (error) => {
          const newAction = options.onError({ action, next, error, getState, dispatch }, options);
          options.onComplete({ action: newAction, next, getState, dispatch }, options);
          return Promise.reject(newAction);
        });

[hotay]

Another thing may help is this only happened after I reinstalled the package recently. (After removed the node_modules/ and then npm install)

[svrcekmichal]

@hotay

this line throw an uncaught exception.

what expception? Can you post it?

[hotay]

@svrcekmichal Nothing much, just an exception:

Uncaught (in promise) Object {type: "STORIES_REQUEST_FAILURE", payload: Error: Request to /stories failed with status code 400.
    at Object.onError (http://localhost:9966…, error: true}error: truepayload: Error: Request to /stories failed with status code 400.
    at Object.onError (http://localhost:9966/dist/bundle.js:48904:19)
    at http://localhost:9966/dist/bundle.js:45482:35type: "STORIES_REQUEST_FAILURE"__proto__: Object

when I replace this line return Promise.reject(newAction); with return newAction; in the local package everything working as expected.

You might notice that the payload seems different from default because I have updated the onError in config so it looks like this:

export const onError = ({ action, next, error }, options) => {
  let errorObject;
  if (error instanceof Error) {
    errorObject = error;
  } else {
    errorObject = new Error(getErrorMessage(action.payload.request.url, error.status));
  }

  const nextAction = createAction(getActionTypes(action, options)[2])(errorObject);
  next(nextAction);
  return nextAction;
};

[hotay]

@svrcekmichal I can make a PR add middleware test for this if you want to.

[hotay]

@svrcekmichal any progress on this?

[svrcekmichal]

People use this middleware 2 ways. Some want axios reject promise to return another reject promise, because this way you can do this:

dispatch(someAction()).then(
    function(successAction) {
        //do something with success
    },
    function(errorAction) {
        //do something with error
    }
);

For this to work you need to use return Promise.reject(error). This is something new in middleware, it was introduced in 1.0.0-rc.0 version and I believe this is the right way to go. What is problem, is that some Promise libraries trigger Uncaught (in promise) because you are not catching error at the end of the promise. I believe you don't need to catch all axios dispatches, because you can use error action to trigger some logic. I believe you want to use it this way:

dispatch(someAction()).then(function(action) {
  //check if action.payload or action.error is present
});

or don't use promise chaining at all. In my opinion we should create new config key named returnRejectedPromiseOnError or something like this. This key should default to false to stay consistent with previous version. This way we can make it work for all whatever way they want.

What do you think about this? @nmaves are you ok with this?

[hotay]

:+1: for the returnRejectedPromiseOnError. In my case, I don't want it to return a reject, because it force me to catch an exception later.

[nmaves]

I am not sure I see a need to change what we have. You currently have three options.

• use a catch() to handle the error where the dispatch happens
• handle the error action in a reducer
• use an axios interceptor to handle it

This gives you a great amount of flexibility. Can you describe a bit more what you are trying to do with your errors?

[hotay]

@nmaves with the current version each time I dispatch an Async action I must to catch afterward and that was not what I expected. e.g:

...
const action = {
  type: 'LOAD',
  payload: {
    request: {
      url: '/stories'
    }
  }
};
...
store.dispatch(action); // at this point you must catch an exception 
                       // otherwise it will throw `uncaught (in Promise)` exception.
                       // Even if you have interceptor you still need to catch at this point, I think.

[nmaves]

What is your plan to deal with this errors? Are you going to handle them in your reducers?

On Sun, May 22, 2016 at 6:58 AM Nguyen Ho Tay notifications@github.com wrote:

@nmaves https://github.com/nmaves with the current version each time I dispatch an Async action I must to catch afterward and that was not what I expected. e.g:

...const action = { type: 'LOAD', payload: { request: { url: '/stories' } } };...store.dispatch(action); // at this point you must catch an exception otherwise it will throw uncaught (in Promise) exception. even if you have interceptor you still need to catch at this point.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/svrcekmichal/redux-axios-middleware/issues/13#issuecomment-220831112

[svrcekmichal]

@nmaves In some promise lib, and even in native Promise in browser, if you don't catch after Promise rejection, you will get uncaught (in Promise) to console and event to Raven. This is not wanted, because you area handling you errors with redux action and not witch .catch. If you still want to use Promise.reject, just add returnRejectedPromiseOnError to your axios config. This is all available in version 1.0.0-rc.1

[nmaves]

I think we really need to look at how this middleware works. It is what I was using before this one. It seems to handle the best of both worlds.

I never had to catch error but I could if I wanted to.

[svrcekmichal]

@nmaves open your chrome browser, open console and type there Promise.reject({}) and you will get uncaught (in Promise). This is received, because you didn't catched promise exception and it was propagated to default error handler.

I only teste their simple example, but as you can see there:

// Request a post from the API with a 1s delay
const getPost = id => ({
  type: 'GET_POST',
  payload: new Promise(resolve => {
    setTimeout(() => fetch(`/api/posts/${id}`).then(response => {
      resolve(response.json());
    }), 1000);
  })
})

they are wrapping fetch request in always successfully resolved promise, so you can't see this error. They are also using fetch, which by default always resolve to success and you have to check status in response object.

If you change code above to this:

const getPost = id => ({
  type: 'GET_POST',
  payload: new Promise((resolve, reject) => {
    setTimeout(() => fetch(`/api/posts/${id}`).then(response => {
      reject(response.json());
    }), 1000);
  })
})

you will see what I am talking about.

[hotay]

Thanks, @svrcekmichal