Error handling for dukcs/fetch about tanto

[kkateq]

@skovhus Corrected above. Might be another issue.

[skovhus]

Same problem with REQUEST_ERROR and CLEAR_ERROR. I'll fix and add a test.

[kkateq]

Corrected. I saw issue with PR, that unstable( not sure that this is it) though. There some section in metadata that tries to render PR metadata info from store that still in fetching progress, if fetch is fast enough - it renders perfectly ;) I am working on this issue by splitting pull request query on few several ones and make each different parts of PR layout to be queried separately(as well as files list) as part of querying files file by file. So that user shouldn't wait until the whole PR fetched to see metadata for example.

[skovhus]

Hm. Then it is also about failing request. The pull request query fails, but no error is propagated to the UI (i.e. error is empty, hence we show the 404 screen on error).

So the error is never set on the state, but the success reducer SET_REPOSITORY is running instead.

[skovhus]

@kkateq Can you describe how you imagined error handling working for fetchActionCreator?

We currently just pass in one callback... Either the callback should take two arguments (node style with data and error) or we need another mechanisms for errors. Or maybe I'm missing something. :)

[kkateq]

Nothing to do with SET_REPOSITORY here it was called before when you browsed the project. When you open PR SET_PULL_REQUEST should be called, if the request is successful, as I can see it's not.

fetchActionCreator wraps action and handle any query isFetching progress and error separately by fetch reducer where you can find a map and get error result by action name. That gives us the advantage to track fetch status and errors separately for any PullRequest partial data query outside pullrequests reducer (where otherwise we would have only one field for error and isFetching which would block partial rendering of PR layout until all the pull request queries finish). Check out ducks/fetch reducer.

[kkateq]

And in the case of exception inside callback, it should be caught inside fetch saga and appear and be processed by fetch reducer.

[skovhus]

I've tracked the bug down.

https://github.com/Unity-Technologies/Tanto/blob/master/src/client/ducks/fetch/index.js#L31

SENDING_REQUEST wrongly replaces the error object.