Next version suggestion

[steida]

I believe the problem with the current design is that promise is returned. In the bigger app, we need to handle global rejections and it interferes with .catch on returned promised.

As far I know, there is the only solution. Action needs an optional callback for success / error. In my app, it's pretty rare since almost everything is optimistic.

Also, take a look at Command Query Responsibility Segregation. http://martinfowler.com/bliki/CQRS.html I'm pretty sure it describes the current design flaw.

// Payload as promise with error handling.
const promiseMiddleware = options => ({ dispatch }) => next => action => {
  const { payload } = action;
  const payloadIsPromise = payload && typeof payload.then === 'function';
  if (!payloadIsPromise) return next(action);
  const createAction = (suffix, payload) => ({
    type: `${action.type}_${suffix}`, meta: { action }, payload,
  });
  payload
    .then(value => dispatch(createAction('SUCCESS', value)))
    .catch(error => {
      dispatch(createAction('ERROR', error));
      if (options.onError) options.onError(error);
    });
  return next(createAction('START'));
};

[tomatau]

I believe the problem with the current design is that promise is returned. In the bigger app, we need to handle global rejections and it interferes with .catch on returned promised.

Why? Your post doesn't really go into this... I've many apps listening to unhandled rejections and this middleware doesn't prevent any functionality (I'm making assumptions about v3, but definitely v2 doesnt interfere). Right now the returned promise can catch to it's hearts content... can you expand on this please.

As far I know, there is the only solution. Action needs an optional callback for success / error. In my app, it's pretty rare since almost everything is optimistic.

Actions should really be serialisable objects that can represent events in your app. Callbacks in actions makes no sense to me and I can't think of any worthwhile use case. Middleware can track actions to add functionality or you can propagate new events or call services directly from your action creators.

Also, take a look at Command Query Responsibility Segregation. http://martinfowler.com/bliki/CQRS.html I'm pretty sure it describes the current design flaw.

CQRS is about using a different "language" or "structure" for reading the model of data to updating the model of data... This middleware is only concerned with providing actions (events) that allow you to update your model (state) any way you please. How you read the data is completely up-to you, reselect is a great option here for creating selectors that you can consume in components, sagas or any service that is interested in getting questions answered about the application state.

So... I don't see anything about how CQRS is related to this middleware.

[pburtchaell]

I think I see the issue you have with catch. If you add middleware to catch all errors, this prevents you from being able to use catch in the action creator. Off the top of my head, I don't have a good solution for you, but I don't think we'd need to change the middleware. This is less about the design of the middleware and more about the design of the middleware catching the errors. Let me know if I am misunderstanding. I prefer to catch errors in action creators, instead of globally; I have not encountered this issue before.

Aside from this, all issues with errors, specifically the known circular reference (and other issues we have encountered in the past) will be resolved with #101. Can you explain other design problems you have?

Switching to callbacks makes absolutely no sense to me.

[steida]

You will see soon that handling all various app errors like no auth, no net, ValidationError, etc. in every action is very tedious, and global rejection handler is very useful. Nevermind. Happy coding.

[steida]

As for CQRS, it's a metaphor, but very relevant. Soon you will realize that change at one place (catch on returned promise) will break code at another place, because returning a promise is a leaky abstraction, trust me or not, but I am pretty sure you will learn it the hard way. Nevermind, thank you and happy codding.

[steida]

As for callbacks, that's because action shall not return anything so the only way to hook async something we need the callback. As I said... happy codding.

[steida]

Just fiy, I was using redux-promise-middleware in pretty popular React dev stack https://github.com/este/este, so my objections were not theoretical.

[pburtchaell]

LOL. Dude, you need to take a chill pill.

[tomatau]

Still don't get it, the catch in an action creator directly on the promise will come into effect before any middleware catch.

dispatch({
  payload: {
    promise: Promise.reject('foo')
     .catch((foo) => {
       console.log('I will log ${foo} first and can still throw')
       throw new Error('bar')
     })
  }
})

const middleware = ({ dispatch }) => next => action => {
  action.payload.promise.catch((bar) => console.log(`I will log ${bar.message} next`)
}

You will see soon that handling all various app errors like no auth, no net, ValidationError, etc. in every action is very tedious, and global rejection handler is very useful.

You don't need to do the catch in the action creator, you can just always do the middleware catch if you want to globally handle stuff... hey, you can even give it a specific list of action types to handle.

I'm using global rejection handling and it's fine... I tend to just deal with the already available _REJECTED action types as they already contain the full error in the payload anyway.

Soon you will realise that change at one place (catch on returned promise) will break code at another place, because returning a promise is a leaky abstraction, trust me or not, but I am pretty sure you will learn it the hard way.

I don't ever really use the promise returned by dispatching a promise action. I find it defeats the point of redux. I'd suggest looking into your reducer design instead of trying to modify errors up stream. If you need to fire off more actions based on rejection then thunks within a .catch are useful from v2. Sagas might be the best solution for you to deal with complex action chains.

Happy coding

Happy coding to you too bud! <3