0

I have this verbatim in a simple script:

Promise.all(links.map(function (l) {
  return Promise.resolve(require(l).r2gSmokeTest())
     .then((v:any) => ({path: l, result: v}));
}))
.then(function(results){

})
.catch(function(e){

}):

the problem is that if the require() call throws an error for whatever reason, the promise chain won't catch it.

is the simplest thing to do to avoid this to wrap Promise.all in a function, like so:

const getAllPromises = function(links){
   return Promise.resolve(null).then(function(){
       return Promise.all(links.map(function (l) {
         return Promise.resolve(require(l).r2gSmokeTest())
         .then((v:any) => ({path: l, result: v}));
}));

it seems to me that maybe Promise.all should have a different API, something like:

Promise.all(function(){
   return values.map(v => whatever);
});

that way any errors can be trapped, if Promise.all is not called within a Promise chain...

2 Answers2

1

Use an async function which can't throw - it will only return a rejected promise when an exception happens:

Promise.all(links.map(async function (l) {
  const v: any = await require(l).r2gSmokeTest();
  return {path: l, result: v};
}))

In general, the expectation for any function that does something asynchronous is to always return a promise and never throw synchronously. Your map callback should adhere to that - and try … catch (e) { return Promise.reject(e); } around the require call if necessary. You might also consider using a helper function that works like Bluebird's Promise.try.

Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • hmmm, but if you do Promise.all().then() in this case, will the `then` catch the promise if an error is raised in the `async` function? I am not sure if it will –  May 07 '18 at 22:27
  • Ok I tested, this works, upvoted, but I am not sure *why*. If you explain more why it works that would be good –  May 07 '18 at 22:32
1

Your require() is happening on the map level outside of Promise context, that's why it gets propagated immediately to the global level, so if you just wrap it into a new Promise it should handle the error as expected:

Promise.all(links.map(function(l) {
    return new Promise(resolve => resolve(require(l).r2gSmokeTest())) // <-- here
      .then((v: any) => ({
        path: l,
        result: v
      }));
  }))
  .then(function(results) {

  })
  .catch(function(e) {

  }):
Karen Grigoryan
  • 5,234
  • 2
  • 21
  • 35
  • I think using `Promise.resolve(null)` to wrap is simpler than creating a new Promise with the executor? –  May 07 '18 at 22:28
  • @Olegzandr Well it depends on the definition of "simple", I thought the "simple" means to fix the problem with as little change to the source as possible. If yours is different, please clarify that. Just bear in mind that `Promise.resolve(arg)` is alias for `new Promise(resolve => resolve(arg))`, and every `.then` is creating a new promise object in chain. That's why in some sense calling `new Promise()` is simpler, because you don't have to create additional promise and don't have to put through the magical placeholder values like `null`. – Karen Grigoryan May 07 '18 at 23:52