Open davidfirst opened 3 years ago
This is, unfortunately, a limitation of JavaScript: https://stackoverflow.com/questions/56644392/passing-an-async-function-as-a-callback-causes-the-error-stack-trace-to-be-lost
The only way to solve this is to:
p-map
to not use new Promise()
. I think the stack trace is preserved if all the async is chained.Thanks @sindresorhus .
I came up with a different implementation that doesn't involve the new Promise
as you suggested. It seems to be working, and the stack trace is fully kept, but the stopOnError
is not implemented.
For my use-case that's good enough, but obviously I can't create a PR with this. I'm pasting it here just in case someone will find it useful.
module.exports = async (
iterable,
mapper,
{
concurrency = Infinity,
stopOnError = true
} = {}
) => {
const results = [];
const iterator = iterable[Symbol.iterator]();
let completed = false;
const runBatch = async () => {
const items = [];
for (let i = 0; i < concurrency; i++) {
const iterableResult = iterator.next();
if (iterableResult.done) {
completed = true;
break;
}
items.push(iterableResult.value);
}
const batchResults = await Promise.all(items.map(item => mapper(item)));
results.push(...batchResults);
if (!completed) await runBatch();
};
await runBatch();
return results;
}
TS version of the above.
export async function pMapPool<T, X>(iterable: T[], mapper: (item: T) => Promise<X>, { concurrency = Infinity } = {}) {
const results: X[] = [];
const iterator = iterable[Symbol.iterator]();
let completed = false;
const runBatch = async () => {
const items: T[] = [];
for (let i = 0; i < concurrency; i += 1) {
const iterableResult = iterator.next();
if (iterableResult.done) {
completed = true;
break;
}
items.push(iterableResult.value);
}
const batchResults = await Promise.all(items.map((item) => mapper(item)));
results.push(...batchResults);
if (!completed) await runBatch();
};
await runBatch();
return results;
}
I looked into this a little bit this morning. Running @davidfirst's examples, I'm able to see the short and long stacktraces. From @sindresorhus's comment, I would have expected that the problem was being caused because we were catching the error on line 59, then passing it to reject
.
I created a simplified version of the pMap
function to try to replicate this, but I get a full stack trace instead:
const fakepmap = async (f) => {
return new Promise((resolve, reject) => {
const next = () => {
(async () => {
try {
resolve(await f());
} catch (error) {
reject(error);
}
})();
};
next();
});
}
const promiseFn = async () => {
throw new Error('stop');
};
fakepmap(promiseFn).then(console.log).catch(console.log);
// Error: stop
// at promiseFn (/Users/me/Source/test.js:19:9)
// at /Users/me/Source/test.js:7:25
// at next (/Users/me/Source/test.js:11:9)
// at /Users/me/Source/test.js:14:5
// at new Promise (<anonymous>)
// at fakepmap (/Users/me/Source/test.js:2:10)
// at Object.<anonymous> (/Users/me/Source/test.js:22:1)
// at Module._compile (internal/modules/cjs/loader.js:1138:30)
// at Object.Module._extensions..js (internal/modules/cjs/loader.js:1158:10)
// at Module.load (internal/modules/cjs/loader.js:986:32)
Any idea what is missing from this test scenario but is present in pMap
that would cause the different stack output?
Any idea what is missing from this test scenario but is present in
pMap
that would cause the different stack output?
I do not know the exact thing but your implementation does not limit the concurrency, maybe that could be an issue. It would be really great to have proper stack traces but so far I have not encountered anything which properly solves this so I assume your implementation omits some step which loses the stack trace but is required for limiting the concurrency.
You could also take a look at p-limit. Its code is a decent bit shorter but suffers from the same issue. It might be easier to figure out which step exactly loses the stack trace in that implementation.
@septatrix,
but your implementation does not limit the concurrency
Why do you think so?
but your implementation does not limit the concurrency
Why do you think so?
My answer was in response to the snippet from @marchuffnagle, not yours
but your implementation does not limit the concurrency
Why do you think so?
My answer was in response to the snippet from @marchuffnagle, not yours
Oh sorry, I missed that.
Both,
console.trace
andthrow new Error
don't show the stack-trace beyond the point p-map got called. In other words, the functions that called before pMap, disappear from the stack trace. As a comparison, the p-map-series doesn't suffer from this issue. It does keep the stack-trace.See the example below, if you run a function that runs the native
Promise.all
, the stack trace shows the function name -runPromiseNative
. Same if you run the functionrunPromisePmapSeries
. However, try to runrunPromisePmap
, and you'll see how the stack trace truncated.I tried node v12.x and v14.x.
Results when running
runPromisePmap
:Results when running
runPromiseNative
.