Exception Handling -- Where are my errors?!

[eschwartz]

I'm running into an into an issue where thrown errors are being caught -- and seemingly released into the ether -- within the Wire JS Promise library (when).

It took a few tries to reproduce, as the original application was fairly complex. But I have a sample now up at eschwartz/when-exceptions. Run index.html and check your console -- expected behavior is to have an error thrown to the console.

The sample is dead simple -- a Foo module loads in a wired context. The Foo#bar method throws an error. The application pulls in Foo with RequireJS, instantiates it, and invokes the bar method.

But the error gets caught inside of the when library, here:

// when.js line 408
function near(proxy, inspect) {
        return new Promise(function (type, args, resolve) {
            try {
                resolve(proxy[type].apply(proxy, args));
            } catch(e) {
                resolve(rejected(e));  // Oh no... my error!!!
            }
        }, inspect);
    }

I am using the most recent versions of wire, when, and meld.

What's happening here? I wouldn't expect errors in this situation to be caught. Can I hook into the promise being referenced in the near method? Can I avoid this situation entirely, and let my errors be thrown?

I'm having a lot of fun with this library, so I hope this can be fixed, or worked-around. Thanks!

[briancavalier]

Hey @eschwartz. Glad to hear you're liking wire so far!

There are several moving parts here: AMD loader, wire.js IOC container, and when.js promises. That's a lot of asynchrony, so things can def get confusing. I'll try to explain what is happening.

All promise libraries catch exceptions and transform them into promise rejections. They have to do this because promises are asynchronous, and onFulfilled/onRejected handlers will always run in different call stacks than where they are registered ... thus, no one, except the promise library, can catch the exceptions. On platforms such as node or WinJS, uncaught exceptions will crash the application (halting the VM).

There are several ways you can handle this situation in wire, depending on how you are using it. If you're using it programmatically, i.e. requireing wire and calling it as a function, you can add a rejection handler to the promise that it returns, and all uncaught errors will bubble out to that promise. For example:

var wire = require('wire');
var promise = wire(...);

promise.otherwise(handleFatalError);

If you are using wire via its AMD plugin, things get trickier because the AMD loader is involved. Wire will report all unhandled exceptions and promise rejections to the AMD loader. This works quite well will cujoJS's AMD loader, curl. I have not tried it recently with RequireJS, but last I checked, it worked there as well.

Now, on to the specific example in your test repo.

I see that in your example, you're throwing an error from within an AMD factory function. That exception can only be caught by the AMD loader--in fact, wire is not involved in that part of the example at all as far as I can tell (it would be involved if the exception were occurring inside test/context.js). To handle that error, you need to add an error handler callback to the bootstrap call to RequireJS. For example, try changing it to the following to see if it helps:

require(['test/foo'], function(Foo) {
  var foo = new Foo();

  foo.bar();
}, function(error) {
  console.error(error);
});

Let me know if that helps!

[eschwartz]

Hi Brian,

Thanks for the response. What's still confusing to me is that the error I'm looking for is not thrown during the loading process, but instead thrown by a method after everything has already been loaded. I think this is the reason I haven't found any way to catch the error when using the the AMD loader plugin -- providing a ReqJS error callback did not work.

I'll show you my ultimate work-around, but it seems like this is unexpected behavior for the library, and something we'd want to fix.

Consider these scenarios

Using the AMD loader plugin - fails (on github)

// foo.js
define([
  'wire!test/context'
], function(wiredContext) {
  // This gets logged with correct context wiring
  console.log('ReqJS loaded Foo module and dependencies with wired context', wiredContext);

  var Foo = function() {
  };

  Foo.prototype.throwError = function() {
    // FAIL -- error does not get thrown when this method is called.
    throw 'Sometimes I feel like no one is listening to me.';
  };

  return Foo;
}, function() {
  // This does not get logged - ReqJS loading successful
  console.log('ReqJS failed to load Foo module dependencies', arguments);
});

Using wire programmatically - succeeds (on github)

// foo.js
define([
  'wire',
  'test/context'
], function(wire, contextObj) {
  var Foo = function() {
    wire(contextObj).then(
      function (context) {
        // This is called, with wired context
        console.log('Wired context programmatically', context)
      },
      function(e) {
        // This is never called -- wiring succeeds
        console.log('Wiring failed', e);
      }
    )
  };

  Foo.prototype.throwError = function() {
    // SUCCESS: Error is thrown when method is called
    throw 'Sometimes I feel like no one is listening to me.';
  };

  return Foo;
}, function() {
  // This is still never called -- ReqJS loads everything fine.
  console.log('ReqJS failed to load Foo module dependencies', arguments);
});

---

What we can conclude from this is that wire is behaving differently as an AMD Loader plugin than as a method to be called programmatically.:

As an AMD loader plugin, it seems like it's wrapping every method of every dependency in a try/catch, as well as the module calling using the AMD loader plugin.

• I should note in my original code, the error was thrown a base class which was a couple dependencies down from the module being wired with WireJS
• It's also notable that the Foo module itself is not being wired at all, but instead is requiring a wired context.

As method to call directly, it does not wrap methods in a try/catch as described above at all.

---

I've tried putzing around the the loader plugin a bit, but haven't had any success.

I would strongly urge that if there is no way to catch errors when using the AMD loader plugin, to suggest within the documentation that ReqJS users not use the AMD loader plugin. Throwing and catching errors is essential to development work, and it can cause a lot of headache if errors you expect to be thrown are caught silently.

Again, thanks for the great library. I'll have to find an opportunity to use the full Cujo suite soon. I really like the way you approach your architecture.

[eschwartz]

I should also note that I tried listening to errors in the main bootstrap module, and none where ever called.

// main.js
require(['test/foo'], function(Foo) {
  var foo = new Foo();

  // This method gets called in all scenarios,
  // but the error the method throws is silently caught.
  foo.throwError();
}, function() {
  // never called
  console.log('RequireJS failed to load main.js dependencies', arguments);
});

[eschwartz]

Unfortunately, my work around is not working.

Using wire programmatically -- fail (on github)

// foo.js
define([
  'wire',
  'test/context'
], function(wire, contextObj) {
  var Foo = function() {
    wire(contextObj).then(
      function (context) {
        // This is called -- wiring succeeds
        console.log('Wired context programmatically', context)

        // -------------
        // FAIL: This error is never thrown.
        // -------------
        throw Error('Ruh-roh...');
      },
      function(e) {
        // This is never called -- wiring succeeds
        console.log('Wiring failed', e);
      }
    )
  };

  return Foo;
}, function() {
  // This is never called -- ReqJS loads everything fine.
  console.log('ReqJS failed to load Foo module dependencies', arguments);
});

---

Any thoughts on where to go from here? There has to be some way to catch errors when using WireJS.

[briancavalier]

@eschwartz Your latest example actually has nothing to do with wire, but it seems that you've been bitten by a unfortunately common mistake when using promises. In your example, the error is thrown, but you've not added promise rejection handler to handle it. I'll try to explain.

The way promise.then works (and this is Promises/A+ standard behavior), is that an error thrown within the onFulfilled callback cause the promise returned by then to be rejected. It does not trigger the onRejected handler that is registered in the same call to then.

Here's a simple example similar to your latest:

//
var promise = ...;
promise.then(function() {
    throw new Error('drat');
}, function(e) {
    // This will NOT be called
});

Here is the way to handle an error in the onFulfilled handler:

var promise = ...;
promise.then(function() {
    throw new Error('drat');
}).then(null, function(e) {
    // This will NOT be called
});

// Optionally, you can use .otherwise(function(e) {...}) instead of then(null, function(e) {..});
//   for the promises returned by wire, since they are when.js promises
var promise = ...;
promise.then(function() {
    throw new Error('drat');
}).otherwise(function(e) {
    // This will be called
});

There are very good reasons that promises behave this way. For example, they guarantee that either the onFulfilled or onRejected passed to the same then will be called exactly once, and never that both are called. For this and other reasons, promises make reasoning about async more like reasoning about sync code. However, it does take some getting used to. I def recommend using otherwise.

If you refactor your example to the following, the "Ruh-roh" will be logged.

define([
  'wire',
  'test/context'
], function(wire, contextObj) {
  var Foo = function() {
    wire(contextObj).then(
      function (context) {
        // This is called -- wiring succeeds
        console.log('Wired context programmatically', context)

        throw Error('Ruh-roh...');
      }).otherwise(function(e) {
        console.log('Wiring failed', e);
      }
    )
  };

  return Foo;
});

[briancavalier]

Have to enabled the wire/debug plugin? It will help (it's not perfect, but does a good job) with error reporting, too, even in cases where you might not add a promise rejection handler:

wire({
    //...
    plugins: [..., 'wire/debug']
});

[briancavalier]

In this example:

// main.js
require(['test/foo'], function(Foo) {
  var foo = new Foo();

  // This method gets called in all scenarios,
  // but the error the method throws is silently caught.
  foo.throwError();
}, function() {
  // never called
  console.log('RequireJS failed to load main.js dependencies', arguments);
});

The error being thrown is the domain of RequireJS. Pinging @jrburke to see if he has any ideas about what might be going wrong.

[eschwartz]

I would be happy to handle a rejected promise, but as far as I can tell, the promise is rejected when an error is thrown is never exposed to me. Am I wrong?

From what I understand of the wire AMD loader plugin, ReqJS will call the errback if the wire method rejects its promise. But the wire promise is never rejected, as you can see from my example. In fact, errors are silently caught even after the wire promise has resolved successfully (see my last example).

Also, we're not talking about errors thrown in loading or wiring. The errors are being thrown in methods of modules which are already wired up, which is why I'm confused that WireJS should be handling these errors at all.

[eschwartz]

Sorry, @briancavalier, I missed your first comment about using otherwise.

Let me give this a go, and I'll let you know how it works. If it does work, maybe the loader plugin should be adjusted to call the errback on otherwise?

[briancavalier]

maybe the loader plugin should be adjusted to call the errback on otherwise

That is an excellent idea! Would you like to submit a PR for it?

[eschwartz]

The otherwise handler was called, but we still have some hurdles:

No way to let errors go uncaught There are cases where I want an error to be be left uncaught, especially during development and QA. The problem is that the otherwise handler is catching thrown errors.

wire(contextObj).
      then(function (context) {
          // This is called -- wiring succeeds
          console.log('Wired context programmatically', context)

          throw Error('Ruh-roh...');
        },
        function(e) {
          // This is never called -- wiring succeeds
          console.log('Wiring failed', e);
        }).
      otherwise(function(e) {
          // This is called
          console.log('Caught error in wire onResolved', e);

          // Good -- we have access to our error.
          // But maybe I don't want to catch it, so
          // let's try throwing it again.

          // FAIL: The error is still caught silently.
          // So there is no way for me to allow an error to go uncaught
          throw e;
        });

otherwise handler is called after AMD module loading

As I've said, we're talking about errors which are thrown after the AMD loaders (ReqJS and CurlJS) have finished loading everything. This means that there is no way for ReqJS to catch the error in a loader plugin.

wire.load = function amdLoad(name, require, callback /*, config */) {
        var errback = callback.error || function(e) {
            // Throw uncatchable exception for loaders that don't support
            // AMD error handling.  This will propagate up to the host environment
            setTimeout(function() { throw e; }, 0);
        };

        wire(name.split(','), { require: require }).then(callback).
                  // By the time we get here, all AMD modules have finished loading
                  // and `callback` has already been called.
                  otherwise(errback);
    };

[briancavalier]

The problem is that the otherwise handler is catching thrown errors.

Remember that uncaught exceptions in future stack frames will crash Node and WinJS, so promise libraries must err on the side of catching them. Whether this is a "problem" or not is a matter of perspective :) During debugging, yes, I agree, it is painful when errors go silent.

If JavaScript had language-level support for promises, the VM could report unhandled rejections, like the one you created above by throwing from within the otherwise. Unfortunately for now, a bit of extra care is probably the only thing that will help that situation.

In the case of RequireJS and using otherwise in wire's AMD plugin, all we can really do is report the error back to RequireJS and hope that it does something with it, which is what is happening now. It's possible we could add a switch to forcibly enable the uncatchable throw which is currently used as a fallback when using an AMD loader that doesn't support callback.error. You could try making that change locally to see if it helps.

Read on, though ...

There are cases where I want an error to be be left uncaught, especially during development and QA

This has been through much discussion and debate in various promise libraries' github issues, and at Promises/A+. Without getting into too much detail, because promises are async, and thus by their very nature have a time component to them, knowing when a promise rejection actually represents a real error is a hard problem. For example, at any instant where you might declare that a promise rejection represents an "uncaught error", in the very next instant, some bit of code could add a otherwise to it and handle the error. So, in the general case, it is a halting problem.

In ES6, JavaScript will be getting native promises. That means that the VM likely will be able to detect and report unhandled rejections (it requires garbage collector hooks to make this work). Until then, yes,

That said, there are things you can do. First is to turn on wire/debug. Second is to turn on when.js's promise monitor. It's still young, but does a pretty good job of both detecting "unhandled" rejections, but then also reporting if/when those become handled.

Give those a try and let me know if they help.

[eschwartz]

Oh boy... this is getting a little too philosophical for a Monday morning. And I see you've given this speech before....

I see what you're getting at though. Basically, a promise can't let an error go uncaught, because maybe someone else wants to handle that error by adding another otherwise handler. So as long as we're using promises to handle asynchronicity, we need to catch all of our errors.

My initial reaction is that WireJS is being a little too opinionated by wrapping everything in a promise -- in my examples, the throwError method is being called after all asynchronous loading is complete (which is why ReqJS has no way to handle the error).

But I really appreciate you working with me through this. I'll may take a shot at that promise monitor -- it looks like it might get me where I need to be.

[briancavalier]

And I see you've given this speech before....

Heh, well, yes, many times in fact. I'm really looking forward to proper VM support for promises. User-space promises seeming to swallow errors is not a great situation, but it's where we are right now.

My initial reaction is that WireJS is being a little too opinionated by wrapping everything in a promise

That's certainly possible. If you probe into wire a bit more, you'll see that nearly everything is async. This allows wire to manage lots of complexity that normally falls to the developer, such as managing DOMReady, order of component initialization, coordinating the timing of startup data fetches, etc. So far, we've felt that those advantages far outweigh the widespread use of promises which, admittedly, can be confusing when using wire initially. However, not funneling these things into promises, in our experience, makes it all much harder to deal with.

It's always great to hear a fresh perspective on this, so I appreciate all the discussion so far. If you come up with ideas for how we can deal with this specific RequireJS-wire situation (heck, you've already come up with one :) using otherwise in wire's AMD plugin), I'm totally open to making improvements.

[eschwartz]

I finally got around to messing with this again, and found a decent workaround for my own project.

I created a new AMD loader plugin for Wire JS:

define(['wire'], function(wire) {
  return {
    load: function(name, require, callback /*, config */) {
      var errback_uncatchable = function(e) {
        setTimeout(function() { throw e; }, 0);
      };

      // Throw uncatchable exception for loaders that don't support
      // AMD error handling.  This will propagate up to the host environment
      var errback = callback.error || errback_uncatchable;

      // If it's a string, try to split on ',' since it could be a comma-separated
      // list of spec module ids
      wire(name.split(','), { require: require }).then(callback, errback).
        otherwise(errback_uncatchable);
    }
  };
});

This results in all errors being thrown, including those inside wired modules.

After hearing you out and doing some learning about the Promise/A+ spec, I'm not sure if this is even a good idea.

It would be nice, though, if the user of the library could configure how these types of errors are handled. With RequireJS, you can pass a configuration object to an AMD loader plugin -- I don't know if this is true of all AMD loader. But you could potentially accept an error callback option in the plugin configuration.

Thanks for your help!

[briancavalier]

Hey @eschwartz, I like both of those ideas: adding a configurable "last ditch" error handler via AMD config (for which we can carefully sniff, in case some loaders don't support it), and using a trailing otherwise.

I'm going to try to get those into an update release sometime in the next week.

Thanks!

[briancavalier]

This just landed in the dev branch: 84bc2ad

[eschwartz]

Finally updated my WireJS version today. Thanks for the commit!

[briancavalier]

@eschwartz Cool! No problem.