fix: properly attribute stack with fileName, removes slow leak

[gabrielcsapo]

Co-authored-by: Brenden Palmer [email protected]

:::DO NOT USE:::

SEEMS LIKE SOURCE MAPS AND VM IS NOT WORKING PROPERLY

Summary

We found a slow leak in fastboot, upon further analysis of the heap we found that the https://github.com/evanw/node-source-map-support in https://github.com/evanw/node-source-map-support/blob/ac2c3e4c633c66931981ac94b44e6963addbe3f4/source-map-support.js#L37-L41 will increase over time by design as errors are created. If we utilize the built in source-map option in node we get better results and node will handle this weak map internally.

Before

stack with incorrect path on disk

After

stack with correct path on disk

[krisselden]

Thank you, nice detective work!

[rwjblue]

I think as written, this is a breaking change right (folks that used to get source mapped errors won't now)? Can we make the change "opt-in" somehow?

Otherwise, we'll have to do a major bump which may be harder to get the change out to folks...

[rwjblue]

I was chatting with @brendenpalmer and @gabrielcsapo and I think we might be able to do some sort of fall back logic like:

1. detect if node >= 14 -- if so -> just call process.setSourcEMapsEnabled(true)
2. if node ~ 12 -> check if CLI flag or NODE_OPTIONS is setup
3. else -> do the old brokenish thing

This would both fix the bug for folks using more modern Node (14+) and continue working (with existing behavior) for folks on Node 12 without the CLI flag on or Node versions older than 12 all without being breaking (as far as I can tell).

[rwjblue]

This has a few issues:

• when we are using process.setSourceMapsEnabled we should not expose any globals
• we should not be changing the name of the global to export (when using Node < 14) see (which uses the old global for exposing source-map-support):

ember-cli-fastboot/packages/fastboot/src/scripts/install-source-map-support.js

Lines 2 to 6 in f944267

	/* globals sourceMapSupport */
	Error.prepareStackTrace = function prepareStackTrace(error, stack) {
	return error + stack.map(frame => '\n at ' + sourceMapSupport.wrapCallSite(frame)).join('');
	};

My suggestion is (basically):

    const sourceMapConfig = process.setSourceMapsEnabled ? null : { sourceMapSupport };

    let sandbox = Object.assign(
      sourceMapConfig,
      {
        console,
        setTimeout,
        clearTimeout,
        URL,
        // Convince jQuery not to assume it's in a browser
        module: { exports: {} },
      },
      globals
    );

This will:

1. do nothing if we are going to use process.setSourceMapsEnabled
2. when we are using the older system, we still expose the source-map-support package at the correct location

[rwjblue]

I've pushed this change, take a look?

[rwjblue]

This seems wrong?

If you look at this:

ember-cli-fastboot/packages/fastboot/test/fixtures/onerror-per-visit/assets/onerror-per-visit.js

Lines 150 to 179 in 3c39627

	;define("onerror-per-visit/routes/slow", ["exports"], function (_exports) {
	"use strict";
	Object.defineProperty(_exports, "__esModule", {
	value: true
	});
	_exports.default = void 0;
	class SlowRoute extends Ember.Route {
	model({
	timeout,
	type
	}) {
	return new Promise((resolve, reject) => {
	setTimeout(() => {
	if (type === 'reject') {
	let error = new Error("slow route rejected after ".concat(timeout));
	error.code = 'from-slow';
	reject(error);
	} else {
	resolve("slow route rejected after ".concat(timeout));
	}
	}, timeout);
	});
	}
	}
	_exports.default = SlowRoute;
	});

Along with these sourcemaps:

https://github.com/ember-fastboot/ember-cli-fastboot/blob/gabrielcsapo/fastboot-source-maps/packages/fastboot/test/fixtures/onerror-per-visit/assets/onerror-per-visit.map

I think the actual path would be onerror-per-visit/routes/slow.js:20:1