acceptable promise pattern for 'LOUD' errors?
Asked Answered
P

2

4

I'm using the RSVP library distributed inside Ember.js and I'm trying to figure out the correct pattern for reporting fatal errors inside a promise -- particularly I want to inform of something that's almost certainly the result of a programming error due to api misuse and I want to do it in a LOUD way. I'm new to promises and javascript in general, hopefully this question makes sense

Here's a simple example (in coffeescript):

doAsync = (arg, callback) ->
  throw new Error('you gave me a way bad arg, I fail for you!')

promiseReturningApi = (data) ->
  return new Ember.RSVP.Promise (resolve, reject) ->
    callback = (err, resp) ->
      if err then reject(err)
      else resolve(resp)
    doAsync(data, callback)

Now lets say I've identified an error that there's no possible way to recover from which occurred inside doAsync -- I want to make sure this error gets reported even if the caller neglected to attach an error handler because it almost certainly only resulted because the caller invoked the api function in an incorrect way

I came across the idea of using setTimeout within a rejection handler to ensure the error gets raised from somewhere even if the caller doesn't attach an error handler to the promise

failLoud = (err) ->
  if err.isProgrammerError
    setTimeout () ->
      throw err
  throw err

promiseReturningApi = (data) ->
  promise = new Ember.RSVP.Promise (resolve, reject) ->
    callback = (err, resp) ->
      if(err) then reject(err)
      else resolve(resp)
    doAsync(data, callback)
  return promise.then(null, failLoud)

Is it considered a reasonable practice to attach such a default error handler to a promise before returning it from my promiseReturningApi? This would allow me to force a stacktrace when the caller does something that can't possibly work -- even though the stacktrace would be a little odd it could make things a bit easier to get started with ...

Even though I called the example promise returning function an 'api' call -- I should add that I'm not writing framework code -- this is rather all within an application. If doAsync were a real-world function, then in my versio of the real-world its pretty likely to be coming from an external party with a new-to-me api -- so it seems pretty likely that I'll misuse it while I'm getting to know it... Meaning I might want to make the pattern something like this

failLoud = (err) ->
  if err?.isProgrammerError
    setTimeout () ->
      throw err
  throw err

promiseReturningApi = (data) ->
  promise = new Ember.RSVP.Promise (resolve, reject) ->
    callback = (err, resp) ->
      if(err) reject(err)
      resolve(resp)
    try
      doAsync(data, callback)
    catch err
      err.isProgrammerError = true
      throw err
   return promise.then(null, failLoud)

I think what this is doing is forcing an exception to be thrown from somewhere any time that my asynchronous function call invocation itself raises an exception -- such an exception would almost certainly be raised during the argument validation phase of the async call which is most commonly going to be the result of my application code passing in something which doesn't make any sense -- and I want to find out about that as soon as I can. Does this seem like a reasonable pattern to follow to aid in debugging promises used in application code in this context?

Producer answered 25/7, 2013 at 19:6 Comment(0)
P
2

New answer --

In this video panel discussion with ember core developers, at one point the developers all share one debugging tip:

http://www.youtube.com/watch?v=L9OOMygo1HI

Tom Dale specifically addresses the issue of swallowed exceptions inside promises and recommends use of the new Ember.RSVP.onerror feature for debugging errors inside promises which would have otherwise gone unreported because no rejection handler was attached.

I think that is the correct answer to my question -- although I don't yet know how to use the RSVP.onerror callback (or in which ember releases its available) ...

Producer answered 1/8, 2013 at 0:25 Comment(1)
In particular I found the solution here -> #17984446Unshackle
A
0

Nice question!

You could get away with the setTimeout within a dev environment. But for production where you log these errors for reporting for instance, You will want to avoid setTimeout as it's non-deterministic. You could end up seeing errors that aren't really accurate, but occurred due to some order in which the setTimeout fired.

You can do this by moving your checks to the first then of the promise, and then returning that thenable instead of deferred directly.

I prefer to use Ember.Deferred instead of Ember.RSVP.Promise. Deferred is a layer over RSVP that has a nicer API. It avoids needing to nest the whole thing inside a callback to Ember.RSVP.Promise. Ember.Deferred provides resolve and reject as methods.

model: function(params) {
  var promise = Ember.Deferred.create();
  var successCallback = function(resp) {
    promise.resolve(resp);
  };

  var errorCallback = function(err) {
    promise.reject(err);
  };

  var thenable = promise.then(function(resp) {
    if (resp.isError) {
      throw new Error(resp.errorMessage);
    } else {
      return resp;
    }
  });

  // some api that takes a callback
  this.callApi(params, successCallback, errorCallback);

  return thenable;
},

Throwing an error at any point in the promise/API will automatically reject the promise. So you don't need to catch and rethrow anything.

This allows for 3 different types of responses.

successCallback with response data.

 successCallback({data: 'foo'});

successCallback with error in response.

 successCallback({isError: true, errorMessage: 'Response data is wrong'});

errorCallback with message.

 errorCallback('Incorrect use of API');

See this jsbin to try this out.

You could clean this up a bit if the API that you call uses promises instead of callbacks. Then you can just chain the thens.

Actually answered 26/7, 2013 at 5:56 Comment(6)
If I understand things correctly, In your example this.callApi can throw an error synchronously (prior to doing anything asynchronous) which would fail in the earliest possible place (in your example at the model() function call site). Whereas in my example, a failure prior to doAsync returning result in the promise entering the rejection state. Ideally the application would find this error and report it some useful way later -- but for the class of errors I want to make very obvious, I'd prefer to have them blown up at the earliest possible place.Producer
I added some application logic to the jsbin you posted inside callApi ... simulating an argument check of the variety which could fail before anything asynchronous needs to happen. In the context of the model hook in ember, I don't have a strong sense of what happens on the side of the framework code which calls model() -- but if this were some other application function which is meant to be called by my own code .. blowing up early would be the right thing most of the time I think. Here's the jsbin modification i made -- jsbin.com/ecarah/2/editProducer
Yes, throwing an error directly in the model hook, will also treat the whole promise as rejected. On ember's side it's the router that does this as a sequence of promises, beforeModel, model, afterModel, etc. They must all succeed else the transition fails.Actually
But there is still something in my mind about this phrase: "Throwing an error at any point in the promise/API will automatically reject the promise. So you don't need to catch and rethrow anything." I think I'm understanding this -- and its great that the ember framework seems to make sure that exceptions in rejected promises returned from the model hook don't go unreported. When it comes to my own code I don't have as much confidence that I'll attach an error handler everywhere I need to ... ember can't protect me if I hide a rejected promise in a promise which I don't return to ember ...Producer
Rejecting promises on errors is part of the Promises/A+ spec. You are correct about the hiding, something to be aware of.Actually
I'm thinking this pattern might make sense failLoud = (err) -> if App.debugMode setTimeout () -> throw err throw err I think it makes sense to me to attach this rejection handler in 'application-oriented' places where I haven't yet fully imagined the way the application could even work if the promise were to reject -- and then as I develop more understanding of how the pieces fit together, I can go back and remove the rejectionHandler or replace it with something more intelligent. Thanks for the tips! (edit: ... Wow can't put code in comments)Producer

© 2022 - 2024 — McMap. All rights reserved.