Breaking: improve JS Error reports

[zazabe]

• more data are reported for JS error:

{
  uid: string,             // unique ID, useful to get previous errors since the page load
  counter: int,            // error counter, since the page load
  previousLog: string,     // previous cm.logger logs (includes cm.debug.log / cm.error.log)
  url: string,             // page url
  error: {
    message: string,          // error message
    type: string,             // error class name
    stack: string,            // error stacktrace
    metaInfo: object|null,    // metainfo (for CM_Exception*)
    source: {                 // error source file / line / column
      url: string,
      line: int,
      col: int
    }
  }
}

• the same data structure is used on the client and on the server side
• JS Error are logged on the server side with the prefix JS Error - + error.message
• js-logger is used as a logger facility
• all logs are always recorded on the client side, with a 5min retention, but console.* is only used when cm.logger.configure({dev: true})

[zazabe]

@vogdb please review

cc @tomaszdurka

[vogdb]

Looks very good. Few notes:

• do we need 'status' => 'ok' in $this->_setContent(json_encode(['status' => 'ok'])); ?

• it might overwrite options

  this._options = _.defaults(options || {}, {

• This name setHandler kind of misleads me cause it sets messages in handlers rather than handler. I think I didn't understand clearly its purpose.

    Logger.setHandler(function(messages, context) {

• Is it ok that we don't pass error.stacktrace in Logger.addRecordError ?

[zazabe]

do we need 'status' => 'ok'

Not really, just thought it's strange to not replying something :)

it might overwrite options

I don't think so, it's the opposite of _.extend(), see http://underscorejs.org/#defaults

This name setHandler kind of misleads

The usage is described here: https://github.com/jonnyreeves/js-logger#default-log-handler-function.

If it's about if (!isDev || (options.dev && isDev))..., i don't think there's another solution with js-logger, because the handler is something defined globally, we can't have multiple Logger instances with specific handlers...
I thought also that the log level could be checked by each handler, but actually, it's the Logger itself that do the check...

Finally, this js-logger is quite limited ;(

Is it ok that we don't pass error.stacktrace in Logger.addRecordError

agree, the stacktrace might be useful, see 1289137

[njam]

Maybe another library to improve stacktraces should be used? Oftentimes the default stacktrace is not very useful, but there are some techniques to improve them.
E.g. https://www.stacktracejs.com/

[njam]

This is quite an important change. We will have a separate log message for every JS error that is recorded. I think it's good, just to keep it in mind.
cc @tomaszdurka

[zazabe]

https://www.stacktracejs.com/

Would be awesome with stacktracejs/stacktrace-gps#4, because in prod, users will not have the sourcemaps.
Maybe it could be done on the server side, with cm-bundler ;) ?

[vogdb]

I don't think so, it's the opposite of _.extend(), see http://underscorejs.org/#defaults

You are right. It might just fill undefined of the original options

lgtm!

[zazabe]

about the stacktrace improvement: #2336

[vogdb]

(deleted)