elmish icon indicating copy to clipboard operation
elmish copied to clipboard

Potential logging improvements

Open TysonMN opened this issue 6 years ago • 3 comments

(A similar issue for Elmish.WFP is https://github.com/elmish/Elmish.WPF/issues/105.)

1. Use strong types to communicate all log event data

Also known as structured logging.

Elmish has two logging levels with corresponding callback functions of type

  • trace: 'msg -> 'model -> unit and
  • onError: string * exn -> unit.

The trace callback already accepts strong types. The onError callback accepts a string as one of its arguments. There are currently just two calls to onError: https://github.com/elmish/elmish/blob/12648f57092ac6c3dca383e1c25995882a646447/src/program.fs#L153 https://github.com/elmish/elmish/blob/12648f57092ac6c3dca383e1c25995882a646447/src/program.fs#L163

In the first call, data of type 'msg has been converted into a string and prefixed with Unable to process the message: . I would prefer if this message were proved to the onError callback as type 'msg Looking at the second call to onError, there is no 'msg involved. There are a few different ways to implement this behavior, such as

  1. onError takes an argument of type 'msg option,
  2. onError takes an argument of type Error = Message of 'msg | Subscription, or
  3. onError is split into onMessageError and onSubscriptionError.

Of course there are other ways to implement this as well.

Resolution of Issue #197 and item 3 below could result in more calls to onError.

2. Implement withConsoleTrace via withTrace

It is currently not possible to implement withConsoleTrace via withTrace. The logging associated with withConsoleTrace is more comprehensive than what is possible using withTrace. I don't want to have to pick between logging to the console with more information or logging to my prefered sink with less information.

Issue #176 is also about withConsoleTrace not being a special case of withTrace...

I assume withTrace is supposed to allow customization of the same output you get with withConsoleTrace...

...but goes on to point out a more serious discrepancy between these two functions. What I am pointing out here is that differences still exist.

3. Improve error logging within dispatch loop

I would like improvd logging in the heart of the Elmish dispatch loop. https://github.com/elmish/elmish/blob/12648f57092ac6c3dca383e1c25995882a646447/src/program.fs#L147-L153

Roughly speaking, there are three ways an exception can be thrown here:

  1. from update,
  2. from setState, or
  3. from the command.

The response to each of those exceptions is very different. Maybe something like

  1. add a unit test for update when passed the same inputs,
  2. create a GitHub issue with the downstream view project (such as Elmish.WPF), or
  3. troubleshoot the problematic impure code.

I would prefer if the logging here immediately distinguished between these three cases.

I recommend resolving (or rejecting 😢) #195 before making changes related to this.

Another somewhat related issue is #186.

TysonMN avatar Nov 09 '19 20:11 TysonMN

  1. You are asking for a change w/o demonstrating the need. As I've mentioned elsewhere logging is a form of data loss and if there's anything we could improve in the regard of capturing the error context it should go in the debugger.
  2. Considering withTrace and withConsoleTrace are more of a courtesy than a core necessity (given how they are implemented), anyone can have their own implementation, so I'm going to decline.

et1975 avatar Jun 13 '20 12:06 et1975

  1. You are asking for a change w/o demonstrating the need. As I've mentioned elsewhere logging is a form of data loss and if there's anything we could improve in the regard of capturing the error context it should go in the debugger.

I no longer like my first suggestion either. I explained my new thinking in https://github.com/elmish/Elmish.WPF/issues/105#issuecomment-635620255, which is about a similar issue for Elmish.WPF.

You are asking for a change w/o demonstrating the need.

"Need" is a strong word. Few things are necessary. I don't think anything I suggested is necessary.

The main goal I am striving for is to improve the efficiency of the development process. I am the technical lead on a project developing an application for a client, and I am responsible for training and then handing off the application to their developers. Especially to that end, but even as my team continues developing the application, I want any runtime problem to be immediately communicated in a clear manner to the developers. That is what this issue and issue #195 is about.

  1. Considering withTrace and withConsoleTrace are more of a courtesy than a core necessity (given how they are implemented), anyone can have their own implementation...

I don't understand. What do you mean by

...anyone can have their own implementation...

TysonMN avatar Jun 13 '20 16:06 TysonMN

The core idea of Program is composition of its functions, write your own update, init, etc and make new Program instance, same way everything else is done - logging, debugger, hmr, etc.

et1975 avatar Jun 13 '20 16:06 et1975