Replace status with boolean error field

[acdlite]

There's been a lot of confusion about status and its purpose. You can read some of the reasoning behind it here: #4 (comment)

I propose we replace status with a boolean error field. If error is true, then the action represents an error, and the payload should (by convention, like with rejected promises) be an error.

[clearjs]

Please describe what value would a restriction to a boolean provide over being any truthy/falsy value?

[clearjs]

Disadvantages:

• not clear how to treat an absent field
• need an additional field for an error object
• makes it more difficult to be compliant

[acdlite]

@clearjs Any false-y value should be interpreted as success, including null, undefined, etc. Only action.error === true indicates failure.

[clearjs]

Good. Then why not go one step further, and allow any truthy value as well? For example, an error object.

[acdlite]

I would keep the error object in the payload. That way you don't get into a situation where an action has both a payload and an error object.

[clearjs]

@acdlite I think I've addressed that here: #4 (comment)

Also, your situation would work just fine with what I propose: error === true would still be truthy, so it would be FSA-compliant. But other usages (like an error in the error field) would be allowed as well without loss of functionality. The end user would decide whichever middleware and action schema fits their needs and preferences.

[clearjs]

By the way, sorry if my tone looks harsh or like I have hard feeling with respect to the outcome, both here and elsewhere. That isn't the intention at all. I'm being a bit pedantic trying to clarify the motivation and making sure that most aspects have been considered.

[acdlite]

An action with error === true is analogous to a rejected Promise. By convention, the payload should be an error object, but nothing's technically stopping you from breaking that convention. However, if you need to dispatch extra stuff, I would suggest catching the original error and dispatching a different action.

[acdlite]

@clearjs No problem, same goes for me! :)

[acdlite]

Changed in v0.6.0

[acdlite]

Please continue any discussion over here: #7

[clearjs]

@acdlite looks like your last comment was intended for some other issue.

[acdlite]

Haha thanks!

[emmenko]

Just a minor note: if we're going with a boolean flag, woudn't be better isError or hasError?

[idolize]

I know this is an old issue, and it seems like the dust has already settled a bit, but I have a concern about errors in the FSA spec:

In the case of an error, the entire action payload is supposed to consist of a single Error object. Unfortunately, this restriction means that there is no way to add any additional payload information to the action. This can be problematic if there is other information required to handle the error, which I believe is a very likely scenario.

For example, if I am a developer and I have an UPDATE_USER action which requires a userId in the payload, my hypothetical Flux store (or reducer) probably needs that userId to update it's mapping of userId -> user data.

Likewise, if the UPDATE_USER action fails, and I dispatch an action with error: true, then I only get a single Error as my payload... how does the store know which user failed to update?

The only solutions currently allowed by the spec (that I can see) are adding the userId information to the Error (which I may not have control over) or adding it to meta, which seems wrong to me because this information is not really metadata at all, but rather a core piece of the action payload.

---

My proposed solution is to change the spec from:

An action whose error is true is analogous to a rejected Promise. By convention, the payload SHOULD be an error object.

to something like the following:

An action whose error is true is analogous to a rejected Promise. By convention, the payload SHOULD contain an error object inside an error field.

This is obviously a breaking change for code using the current spec; it would require error handling middleware to mirror the payload object with an additional error field:

Ex: dispatch({ ...prevAction, error: true, payload: { ...prevAction.payload, error: errorObj } })).

And in addition, the handler of the action would have to access the error object via action.payload.error instead of action.payload.

However, the result would be much friendlier to Flux developers in my opinion.

[tappleby]

I dont see much value in having both action.error and action.payload.error.

If we relaxed the error requirement to just contain a truthy value I think that be enough:

dispatch({
    ...prevAction
    error: {
        message: "Error saving user"
    }
});

[idolize]

@tappleby

Perfect. That idea also came to me before bed last night -- seems much simpler than requiring both, and it allows us to use the payload for other things :)

[tappleby]

I realized after posting that there was previous discussion about making error truthy, @acdlite gave some reasons for why error should be in payload but I think its worth revisiting.