Git Product home page Git Product logo

Comments (19)

bryanlarsen avatar bryanlarsen commented on July 27, 2024 8

The error design of Flux Standard Actions is completely unworkable for us; we've decided to not use Flux Standard Actions for that reason.

  1. As mentioned, the 'error' boolean is redundant. For us, an error is any action that has a type that ends with 'ERROR'.
  2. We almost always require the original payload when handling error actions. For example, when we have an error updating a model, we update our state to contain the list of field that have not been successfully updated. It's certainly possible to put the original payload into meta.payload or payload.payload or something like that, but that's just silly. In our "FSA", the payload property of an error action is always the original payload.
  3. Given that most error objects are the result of HTTP actions, we have a standard field that contains the HTTP status code. Not all errors have to have this field, but when they do, it's in a standard place.
  4. We make the Error object required rather than optional, and put it in the error property of the action.

from flux-standard-action.

migueloller avatar migueloller commented on July 27, 2024 2

I think the original intent of adding error as a boolean property can be deduced from the example in the readme.

{
  type: 'ADD_TODO',
  payload: {
    text: 'Do something.'  
  }
}

{
  type: 'ADD_TODO',
  payload: new Error(),
  error: true
}

There are no redundancies in this example.

Due to the way people use Redux, though, it starts to get redundant.

{
  type: 'ADD_TODO_REQUEST',
  payload: {
    text: 'Do something.'  
  }
}

{
  type: 'ADD_TODO_SUCCESS',
  payload: {
    text: 'Do something.'
  }
}

{
  type: 'ADD_TODO_FAILURE',
  payload: new Error(),
  error: true
}

Perhaps the correct approach would be to separate app concerns from asynchronous concerns and instead of doing request -> success | failure, it's better to do request -> success (triggers action<foo>) | failure (triggers error<foo>), where action<foo> is equivalent to { type: 'foo' } and error<foo> is equivalent to { type: 'foo', error: true } with their respective payloads.

The only issue this brings are optimistic updates. This could be solved, though, by triggering actions that queue the change when the request is made and then committing when it succeeds or reverting if it fails.

from flux-standard-action.

tomatau avatar tomatau commented on July 27, 2024 1

+1 for error property being any object with a message property.

You can serialise an Error object:

JSON.stringify(new Error("my message"), ['message'])
// "{"message":"my message"}"

You can even serialise the stack if you so wish...

Having an error boolean flag to me seems like a duplication of action information.
It's good to have a place for a standardised error format that can hold general information.
Storing state specific IDs for failure in the meta data also seems like the wrong place.

For example, an action sequence that performs a batch update with optimism.

{ type: BATCH_ITEM_UPDATE_PENDING, payload: {items[]}, meta: {actionId} }
{ type: BATCH_ITEM_UPDATE_SUCCESS, payload: {successful_item_ids[]}, meta: {actionId} }
{ type: BATCH_ITEM_UPDATE_FAIL, payload: {failed_items[]}, meta: {actionId} }

The meta property contains an id specific to the action sequence -- not the state changes.
The payload contains multiple ids (and more), for any items that failed and needs reverting from the optimistic update (or whatever else -- maybe error messages in the app?).

from flux-standard-action.

thedumbtechguy avatar thedumbtechguy commented on July 27, 2024 1

I came from SO for this same issue.

When an error occurs, it IS an action. It is not an action with an error.

I forsee conditional logic sprinkled all over your action handling code checking for an error property when you could simply be handling an error action.

If you are using something like redux, you can have a dedicated reducer handling errors.

A better design would have been to standardize the error payload so handling is streamlined.

Another thing is properties that 'MAY' exist. How is that even standard then? More conditional logic for me.

An action should have ONLY two properties. A TYPE and a PAYLOAD. Everything else, error messages, meta data etc should be in the PAYLOAD. That is a standard.

from flux-standard-action.

cesarandreu avatar cesarandreu commented on July 27, 2024

I think the reasoning might be so that you can serialize the action (which requires converting the Error object into a plain object) but still be able to convert the serialized action into its original state.

from flux-standard-action.

gajus avatar gajus commented on July 27, 2024

Ah, OK. It is not a problem then. I was working on the Canonical Composition Action spec. If you do not require that object is an instance of Error and instead specify certain traits (e.g. it must have message property), then object can be serialised and unserialised as needed.

By the way, I am happy if FSA copy from CCA everything. I am only trying to improve the standard. I have little hope for replacing the standard.

from flux-standard-action.

pbomb avatar pbomb commented on July 27, 2024

I'm struggling a bit with this design as well. Our application has the typical pattern of fetches dispatching REQUEST, SUCCESS and FAILURE actions. For all of these actions, the payload always contains the contextual information required to identify which part of the state tree its for. If the FAILURE action is supposed to have the error property set to true and the payload property set to the error object, then the contextual properties don't have a home.

This is unless contextual information is always supposed to go into the meta field. My questions is related to the question in issue #18 asking what is supposed to go into the meta field. If contextual info should go there, then the REQUEST action would not have a payload, but only have meta. Can someone explain how this typical fetch pattern works with Flux Standard Actions?

from flux-standard-action.

mlegenhausen avatar mlegenhausen commented on July 27, 2024

Forcing the assignment of error to the payload property seems to force you to use custom error objects, that contain all necessary information to describe the error context.

from flux-standard-action.

gajus avatar gajus commented on July 27, 2024

Which is a good thing.

On Oct 15, 2015, at 10:42, Malte Legenhausen [email protected] wrote:

Forcing the assignment of error to the payload property seems to force you to use custom error objects, that contain all necessary information to describe the error context.


Reply to this email directly or view it on GitHub.

from flux-standard-action.

tomatau avatar tomatau commented on July 27, 2024

I agree that It's good to enforce standards and common structures but I don't think the payload is the best place for that when handling an error action.

Right now we already force the error field to be a boolean, which is clearly limited in the information it can communicate -- so seems like a better place to enforce custom error objects. Then the payload doesn't need to conform to a structure when in an error action and can be free to contain more data about the error-action.

from flux-standard-action.

mlegenhausen avatar mlegenhausen commented on July 27, 2024

Using an error flag is more explicit in term of what is possible with javascript. If you want to use a falsy value as your "error object"/payload you are unable to determine if your action is an error or not.

Example:

// works
{
   error: {
      status: 404
   }
}

{
  error: 'Error message'
}

// does not work
{
  error: ''
}

// works again
{
  error: true
  payload: { status: 404 }
}

{
   error: true
   payload: ''
}

from flux-standard-action.

tomatau avatar tomatau commented on July 27, 2024

@mlegenhausen -- you can have a falsy value in your payload if you conform to the error value should always have a message property -- like an Error object, then everything else that doesn't have a message property can be ignored.

No-one here is suggesting that passing an empty string (or any other ambiguous falsy value) should be used as an error value.

from flux-standard-action.

mattkrick avatar mattkrick commented on July 27, 2024

I agree with where this is going. If there's an error, it's in my action type & that action creator can handle the error. What I don't like is having the error itself in the payload instead of the action that caused the error (or the multiple failed items if it's a batch job). I say let error be an Error object & for those edge cases where the action type doesn't have an ERROR/FAIL suffix, they can action.error instanceof Error (after a rehydrate, if serialized).

from flux-standard-action.

m-sanders avatar m-sanders commented on July 27, 2024

I agree with the general sentiment of this issue.

I personally am using something akin to Error Handling in Nodejs guidelines, where only ever one property of data or error is set, error is always an instance of Error with an appropriate computer friendly name and a human friendly message. Any additional context objects can be attached to Error and should consistently exist for all instances of Error with the same name. On success data is a free for all but I try to document all relevant data on the action. I am using data over payload - both are equally vague but data is less keystrokes!

Anyway, I am sure there is an XKCD mentioning something about competing standards....

from flux-standard-action.

MarkMurphy avatar MarkMurphy commented on July 27, 2024

👍 I concur also. Ditch the boolean on the error attribute and store the error there.

from flux-standard-action.

jchook avatar jchook commented on July 27, 2024

Consider this ideal use case where an AJAX request has 3 prongs:

  1. initial dispatch({ type, payload })
  2. success dispatch({ type, payload, meta: { serverResponse }})
  3. error dispatch({ type, payload, error, meta: { serverResponse }})

Wouldn't it make the most sense to have the payload consistent across all three in order to tell what parameters led to error / success? Unless I keep the original action in the action-creator scope, then put it in meta (yo dawg I heard you like actions), I cannot see what the payload of the initial action was.

from flux-standard-action.

tomatau avatar tomatau commented on July 27, 2024

The parameters causing an error can be put into an Error object -- along with many other things.

I would assume it's more common for server responses to be placed in payload, not the meta object -- which makes the payload inconsistent already.

from flux-standard-action.

sagiavinash avatar sagiavinash commented on July 27, 2024

we can have a hasError flag in meta object and payload be the error object.

from flux-standard-action.

thedumbtechguy avatar thedumbtechguy commented on July 27, 2024

@migueloller

Exactly my point.
What you should be doing is standardizing the payload for a given action your application.
That way, your app knows how to react when ever it sees that action type.

What happens when you decide that for a certain error, you want to do something different for example?

You

from flux-standard-action.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.