Skip to content
This repository has been archived by the owner on Apr 21, 2020. It is now read-only.

Add error handling mechanism [3h] #97

Open
amitaibu opened this issue May 3, 2017 · 4 comments
Open

Add error handling mechanism [3h] #97

amitaibu opened this issue May 3, 2017 · 4 comments
Assignees

Comments

@amitaibu
Copy link
Member

amitaibu commented May 3, 2017

No description provided.

@amitaibu
Copy link
Member Author

amitaibu commented May 3, 2017

@rgrempel added some great thoughts in another (private) thread, so copied (and adapted links) here:

Here are a few initial thoughts.

Generally speaking, I think it's nice to delay converting stuff to strings until as late as possible. So, I wouldn't necessarily demand that the error type have some text up front ... instead, I think it would be better to model whatever we want to model about the error context, and then have some function somewhere that turns it into text (at the appropriate moment). For an example, see viewError, but of course we'll ultimately want something somewhat more sophisticated than that.

Now, I think there are at least three general classes of error, as far as the UI goes.

  • One is the "we would be showing something here if we had it, but an error occurred and we don't have it at all."

    This is a relatively easy case, I think, as far as UI goes. In the data model, we'll typically have a RemoteData to represent the data that we may or may not have. So, if we don't have it, we display some kind of error message where we would otherwise display the data. This is a pattern we frequently use ... see this for instance.

  • Another case would be "we have it, and are showing it here, and tried to update it, but something went wrong".

    Ultimately, what I think we really are going to want for this is a slightly more sophisticated version of RemoteData that can represent this situation. At the moment, the RemoteData either has your data (with no error recorded), or it records the failure (forgetting your data). But, sometimes, we logically want both -- that is, we logically want to remember the data we had, but also record that we tried to update it and got an error. So, really, we need either another wrapper around RemoteData, or just a variation of RemoteData that is conscious of updates.

Consider, for instance, something like this. RemoteData is currently defined this way (at least in Elm 0.17 -- may be slightly different in 0.18):

type RemoteData e a
    = NotAsked
    | Loading
    | Failure e
    | Success a

One way to improve on this would be something like this:

type RemoteData e a
    = NotAsked
    | Loading
    | Failure e
    | Success a a (RemoteData e a)

So, it's the Success case that would change.

  • The first a would be the data successfully returned from the server.
  • The second a would be the local data, possibly modified.
  • The recursive RemoteData e a would track our attempt to update the server with our second a. So, it would start as NotAsked ... i.e. we don't have an update request in progress yet. If it fails, we'd note the failure, but we'd still know both what the server thinks the data is, and what locally we wanted the data to be. If it succeeds, we'd collapse it back down to Success newValue newValue NotAsked ... i.e. we'd record that the server data now matches the local data, and there's no update in progress any longer.

Or something like that -- I think that's not quite perfect. But, the general approach is, I think, correct -- i.e. that we ultimately need to track some extra information about updates in progress, which RemoteData doesn't currently do.

Now, given such a type, we then have everything we need to display something nicely in the UI -- exactly how to do that would depend on the context. For instance, we could immediately show the new value, but with some styling to indicate that it hasn't been sent to the server yet. Then, when the update is in progress, we could show some different styling. And if it error's, we could show some styling for the error -- perhaps an outline, and tooltip to explain, or whatever (for instance, even a popup to "retry", since we track the value we were trying to set). Anyway, once the data model is correct, it makes it easy to do what makes sense in the UI.

  • A third case would be errors which occur but which don't relate to something we'd otherwise be showing in the UI. Or, to put it another way, errors that don't naturally relate to something already in our data model.

    For things of that kind, I think we'll want to define an app-specific AppError type (or types) which would include a time, and then cases for specific error types. Then, we'd put a List AppError in the data model at appropriate points, so we can make a UI to display those errors. It might be a global UI (i.e. for all errors in the app), or possibly sub-divided (if it makes sense to indicate some errors in some contexts and others in others). So, the UI might be a kind of admin panel, or it might be a little icon that pops up if there are "fresh" errors (with a number of new errors), or whatever. This is sort of what I was gesturing towards with the pusher monitoring stuff, particularly the viewErrors button which would bring up a table of errors.

Anyway, those are a few thoughts.

@amitaibu
Copy link
Member Author

amitaibu commented May 3, 2017

@rgrempel I had different thoughts about this, which involved a list of errors. But then I realized we're not just dealing with errors. Showing for example a Saved item successfully can be the job of this same LogHandler module.

type LogTypes 
    = VisibleLog WebData (WebData -> String)  -- 2nd argument produces the visible string out of the data
    | HiddenLog WebData

type alias Logs = List LogTypes

So child components can return something like:

update : Msg -> Model -> (Model, Cmd Msg, Maybe Logs)

I think the difference in our approach is the separation of concerns. That is, once a child component has returned the log item they no longer care about it. That is, the data itself is separated from the need to log it.

Then a higher component will take the List of logs, process it, and trim it down. We get some duplication of data, but keep the WebData type still simple to interact with.

What do you think?

@rgrempel
Copy link
Contributor

rgrempel commented May 3, 2017

Yes, that's consistent with my third case ... that is, errors which don't necessarily relate to some data we're currently displaying.

But, you're right that it isn't necessarily just errors ... we might want to keep a "history" of what happened, so successes too. Events, you might say.

So, you could imagine wanting both ... that is, you could imagine wanting an UpdateableData that tracks errors relating the latest client/server interaction, along with a logging system that simply records events, positive and negative.

However, in defining the EventType, I wouldn't focus initially on hidden vs. not hidden or how to represent it as a string. There is a great article on the web somewhere -- sadly, I can't find it now -- which argued that we're doing logging all wrong by reducing structured information to strings too early. Basically, it argues that we should type the event according to its natural characteristics, so that it is genuine data that we can use in a variety of ways -- without having to re-parse a string to pick out what happened. (So, log files shouldn't be lines of text, but instead rows saved to a database, or at least JSON or something with actual structure).

Now, we'll want one or more EventType -> String functions, possibly, for display, or in fact EventType -> Html. But we don't need to store the resulting string ... we can compute it based on whatever the natural, typed, information about the event is. (In fact, in some cases our Msg types are the typed information about what happened ... they are, after all, the way to make things happen. But, we wouldn't necessarily want to record every Msg, of course, and sometimes what we want to record may be a bit distinct from the Msg).

Similarly, we shouldn't really decide up front whether the event is "visible" or not -- it might be visible in one UI context and not another, for instance. We should just include whatever real data we need in order to decide later whether to make it visible in one context or another.

In terms of how to structure the generation and capture of event logs in an app, your update signature would work ... the parent would be responsible for dispatching the returned events somewhere appropriate (perhaps to its parent, possibly mapping it into it's own Event type). The other way to do it would be to define messages that record a log entry, and have the parent dispatch those messages. But if the only relevant message is "add this entry to the log" then I suppose returning a List EventType is just as well -- the message is, in a sense, implied.

@amitaibu amitaibu self-assigned this Oct 1, 2017
@amitaibu amitaibu changed the title Add error handling mechanism Add error handling mechanism [3h] Oct 1, 2017
@amitaibu
Copy link
Member Author

amitaibu commented Oct 1, 2017

I'm going to tackle this one, to add the simplified error mechanism we have in other internal projects, which make errors more visible during development.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants