Add improved logging to Promises, particularly during error catching. about deftjs HOT 5 CLOSED

I'm considering adding the following "info" level logging to Deferred and Promise operations:

• for when then() is called:
  • immediately at the start of that call:
    Registering callbacks with Deferred.
  • just before then() returns the new Promise:
    Returning a Promise of the transformed result.
• for the Promise returned by a call to then()
  • when that Promise returns a Promise:
    Promise will be completed based on the Promise returned by its callback.
  • when that Promise returns a value:
    Promise resolved with the value returned by its callback: <value>
  • when that Promise throws an Error:
    Promise rejected with the Error returned by its callback: <error.stack>
• for any Deferred
  • when update() is called:
    Deferred updated with progress: <progress>
  • when resolve() is called:
    Deferred resolved with value: <value>
  • when reject() is called:
    Deferred resolved with error: <error>
  • when cancel() is called:
    Deferred cancelled with reason: <reason>

Does this seem like an appropriate level of logging coverage?

Additionally, in order to aid developers in debugging logic that relies on Promises and Deferreds, I'm considering adding an optional id property to Deferreds. When specified, the logging messages above would be modified to include that identifier.

Thoughts?

from deftjs.

The coverage seems fine to me. The identifier should definitely be useful. Maybe it would help to use a different terminology for the Promise created and controlled directly by the user code, and the Promises created by calls to then(). Maybe "Delegate Promise" ?

The "Promise rejected with the Error raised by its callback" message would address issue #59. IMO however, when an error is raised in a callback and there is no registered failure callback to notify, that should be considered an uncatched exception, and so it would be appropriate to log it with an "error" level. That's something you want to see in red in the console, especially if it happens in a production environment where ext-dev has not been included.

Was it really a good idea for the spec that the only way to pass an error to the following promises is by throwing it? Returning (instead of throwing) an instance of an Error object would spare that dangerous catch-all block. Mixing managed errors and unexpected exceptions seems a bit messy to me. I am curious to know what you think about that?

from deftjs.

In practice, this approach has proven much too verbose to be useful.

• Internal Promises used within the Promise helper methods emit detailed logging that is irrelevant to user code.
• Often the Promise you care about is one that was created as a result of then() chaining, and that Promise can't easily be tagged with an identifier.
• Valuable logging gets lost in the noise.
• In a production environment where ext-dev has not been included, real errors are swallowed and go unreported.

Deft JS 0.9 will implement a new helper method available on all Promises called log(). It can be used anywhere in a Promise chain and will log the resolution or rejection of the preceding Promise, passing the result through to allow for further chaining and transformation. log() will support an optional name parameter that is incorporated into the resulting logging message.

Example:

getUser(currentUserId)
  .log("Promise of the current User")
  .then( function (user) {
    return Promise.map(user.friendIds, getUser)
  })
  .log("Promise of a User's friends");
  .done();

In this contrived example, we get the details for a user and use Promise.map() to get the details for all of the user's friends.

The log output for fulfilled Promises would look like this:

[I] Promise of the current User resolved with value: <value>
[I] Promise of a User's friends resolved with value: <value>

The log output for rejected Promises would look like this:

[I] Promise of the current User rejected with error: <error>
[I] Promise of a User's friends rejected with error: <error>

Also note the use of done(), which will also be part of Deft JS 0.9. By placing the done() at the end of this chain, any unhanded rejections earlier in the Promise chain will throw an Error. See #83 for more details.

from deftjs.

This feature and its associated unit tests have been implemented in the promises_aplus branch and will be part of the Deft JS 0.9 release.

from deftjs.

Deft.Promise::log() is now available as part of the new Promises/A+ compliant rewrite of Deft JS's Promises.

See also: #82

from deftjs.