microsoft / node-uwp

Enables Universal Windows Platform (UWP) API access for Node.js (Chakra build) on Windows 10.
MIT License
152 stars 26 forks source link

Exceptions swallowed by promises #17

Open hgwood opened 8 years ago

hgwood commented 8 years ago

Reprising your sample and introducing an obvious mistake shows that errors don't seem to escape promise handlers:

var uwp = require('uwp');
uwp.projectNamespace("Windows");

Windows.Storage.KnownFolders.documentsLibrary.createFileAsync(
  "sample.dat", Windows.Storage.CreationCollisionOption.replaceExisting)
  .done(
    function (file) {
      undefinedFunc();
    },
    function (error) {
      undefinedFunc();
    }
);

Running this shows no error, it just runs eternally because uwp.close() is never called. It makes it hard to detect errors, especially programming ones.

I read about WinJS.promise.onerror, but I'm not sure how it relates to the uwp module. Any pointers?

kunalspathak commented 8 years ago

Thanks @hgwood for reporting this. I will investigate this and get back to you.

kunalspathak commented 8 years ago

@hgwood , I would go with .then().done() approach as given below. I verified that this lets you catch the errors and close the uwp.

var uwp = require('uwp');
uwp.projectNamespace("Windows");

Windows.Storage.KnownFolders.documentsLibrary.createFileAsync(
  "sample.dat", Windows.Storage.CreationCollisionOption.replaceExisting)
  .then(
    function (file) {
      undefinedFunc();
    },
    function (error) {
      undefinedFunc();
    }
  ).done(
    function (result) {
    },
    function (error) {
      console.log('handle all the errors');
      uwp.close(); // close uwp
    }
 );
hgwood commented 8 years ago

That's a nice work-around thanks, but wouldn't you say the original problem is a bug? Shouldn't done make its callback throw freely? Isn't that its main difference to then? I guess my real question is: could it be fixed in future versions?

dotnetCarpenter commented 8 years ago

Sorry if I add clutter to the conversation but it surprise me to see the .done() method in a Promise outside jQuery. There is no mention of a .done() method in the JS language specification.

Following the links from MSDN leads to a deprecated test suit for Promises/A.

AFAIK the list is:

Promise
Promise.all
promise.catch
Promise.prototype
Promise.race
Promise.reject
Promise.resolve
promise.then

Is the exact behavior of .done() documented anywhere?

dotnetCarpenter commented 8 years ago

Off the top of my head, I can only think of the following similar cases for JS Promises:

// Thenable throws before callback
// Promise rejects
var thenable = { then: function(resolve) {
  throw new TypeError("Throwing");
  resolve("Resolving");
}};
var p1 = Promise.resolve(thenable);
p1.then(function(v) {
  throw new Error("Should not be called"); // not called
}, function(e) {
  console.log(e); // TypeError: Throwing
});

// Thenable throws after callback
// Promise resolves
var thenable = { then: function(resolve) {
  resolve("Resolving");
  throw new TypeError("Throwing");
}};
var p2 = Promise.resolve(thenable);
p2.then(function(v) {
  console.log(v); // "Resolving"
}, function(e) {
  throw new Error("Should not be called"); // not called
});
hgwood commented 8 years ago

@dotnetCarpenter done is present in Bluebird, with the same semantics, but its usage is discouraged.