search

YahooArchive / fluxible-router

MOVED TO FLUXIBLE REPO
104 stars 26 forks source link

call done with Error object #31

Closed pgherveou closed 9 years ago

pgherveou commented 9 years ago

following up with https://github.com/yahoo/fluxible-router/issues/30

yahoocla commented 9 years ago

Thank you for submitting this pull request, however I do not see a valid CLA on file for you. Before we can merge this request please visit https://yahoocla.herokuapp.com/ and agree to the terms. Thanks! :smile:

mridgway commented 9 years ago

Nice catch. Can you also add the statusCode = 404 to the error object?

pgherveou commented 9 years ago

Alright we should do it for the error500 as well then? Do you prefer to dispatch directly the error object or keep it as it is and dispatch an error object with status and message cloned from the dispatched payload?

mridgway commented 9 years ago

Yes, we should do both actions. We do not use an Error object in the dispatch because it will lose some information when being dehydrated/rehydrated (call stack for instance). This ensures that components are not using data that could be different between client and server.

pgherveou commented 9 years ago

It would be nice to add an Error type check in the dispatchr module as well at dev time to catch this kind of errors

yahoocla commented 9 years ago

CLA is valid!

coveralls commented 9 years ago

Coverage Status

Coverage increased (+0.0%) to 99.43% when pulling dee188504c1f6ddfabd27b10290b96ab6d255d4b on pgherveou:patch-1 into e10cf5f8487ffe508762bda4f5055414aad5a5a1 on yahoo:master.

mridgway commented 9 years ago

Looks good! Thanks!