Stack trace is lost from the point the code uses p-map

[davidfirst]

Both, console.trace and throw 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 function runPromisePmapSeries. However, try to run runPromisePmap, and you'll see how the stack trace truncated.

I tried node v12.x and v14.x.

const pMap = require('p-map');

const pMapSeries = require('p-map-series');

async function promiseFn() {
  throw new Error('stop')
}

async function runPromiseNative() {
  await Promise.all([promiseFn()]).then(() => { console.log('completed') }).catch((err) => console.error(err));
}

async function runPromisePmap() {
  await pMap([promiseFn], fn => fn()).then(() => { console.log('completed') }).catch((err) => console.error(err));
}

async function runPromisePmapSeries() {
  await pMapSeries([promiseFn], fn => fn()).then(() => { console.log('completed') }).catch((err) => console.error(err));
}

// runPromiseNative();

runPromisePmap();

// runPromisePmapSeries();

Results when running runPromisePmap :

Error: stop
    at promiseFn (/Users/davidfirst/teambit/bit/pmap.js:6:9)
    at /Users/davidfirst/teambit/bit/pmap.js:14:33
    at /Users/davidfirst/teambit/bit/node_modules/p-map/index.js:57:28

Results when running runPromiseNative.

Error: stop
    at promiseFn (/Users/davidfirst/teambit/bit/pmap.js:6:9)
    at runPromiseNative (/Users/davidfirst/teambit/bit/pmap.js:10:22)
    at Object.<anonymous> (/Users/davidfirst/teambit/bit/pmap.js:21:1)
    at Module._compile (internal/modules/cjs/loader.js:999:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1027:10)
    at Module.load (internal/modules/cjs/loader.js:863:32)
    at Function.Module._load (internal/modules/cjs/loader.js:708:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:60:12)
    at internal/main/run_main_module.js:17:47

[sindresorhus]

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:

1. Find a way to refactor p-map to not use new Promise(). I think the stack trace is preserved if all the async is chained.
2. Or get the stack trace before the await, then afterward, merge the stack traces somehow. We did something like this in Got: https://github.com/sindresorhus/got/blob/462bc630015064fa4ad4358cf28d24f95e1c958b/source/core/index.ts#L1225-L1237

[davidfirst]

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;
}

[marchuffnagle]

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?

[septatrix]

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.

[davidfirst]

@septatrix,

but your implementation does not limit the concurrency

Why do you think so?

[septatrix]

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

[davidfirst]

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.

[davidfirst]

In case you already have lodash in your project, consider this simplified version which also fixes the stack-trace.

import { chunk } from 'lodash';

export async function pMapPool<T, X>(
  iterable: T[],
  mapper: (item: T) => Promise<X>,
  {
    concurrency = Infinity
  } = {}
) {
  if (concurrency === Infinity) {
    return Promise.all(iterable.map((item) => mapper(item)));
  }

  const results: X[] = [];
  const chunks = chunk(iterable, concurrency);

  for (const currentChunk of chunks) {
    const batchResults = await Promise.all(currentChunk.map((item) => mapper(item)));
    results.push(...batchResults);
  }

  return results;
}