Allow deserialization error handling in fetch

[glademiller]

The Fetch module works great but it has a couple of expects where I would hope to have a Result that I can handle to say something went wrong with the request. Today you could do the string and Json deserialization manually and handle the errors but it would be nice if the built in functionality allowed for it.

[jesskfullwood]

There was some discussion of this in the original PR. It should obviously be done but it seems the implementation will need some thought.

[glademiller]

I totally agree. A solid solution wasn't super obvious or I would have opened a PR. Just for the sake of furthering the discussion my expectation was to be able to handle the error case and send a Msg just like I did for the success case.

[glademiller]

Looking at the discussion it seems to revolve primarily around how to handle the two different error types. For my particular use case a new error type that has From implemented for both would be enough. What other options would seem to fit the situation? If an ideal solution could be hashed out I might be able to spare some time to put up a pr.

[sapir]

Futures are now returned from update, and their result needs to be Result<Msg, Msg>, so the error case is now passed as a Msg back to the update function.

[David-OConnor]

@glademiller does sapir's latest PRs/the 3.0 release resolve this?

[glademiller]

I'm assuming these are pull requests that have already been merged. If so then no it doesn't the big issue is the two expect calls when deserializing json and converting a string. I need to be able to handle a contract violation by the server in a graceful way right now it would simply panic. It does sound like the change sapir made is an improvement though. It just doesn't solve the problem I'm seeing. Thanks, Glade

…

On Thu, Mar 21, 2019, 23:23 David-OConnor ***@***.***> wrote: @glademiller <https://github.com/glademiller> does Sapir's latest PRs/the 3.0 release resolve this? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#36 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB6chBm61VDLatLWI0iSgT4QsxgVvSteks5vZGi2gaJpZM4aXSxL> .

[MartinKavik]

I'll try to fix it.

[MartinKavik]

@glademiller Merged PR #118 should resolve your problems - could you provide feedback or/and close the issue?

[glademiller]

I haven't dug too deep but looking at the code I believe this solves the issues I had. Thank you!