lib: avoid mutating Error.stackTraceLimit when it is not writable

[aduh95]

This PR aims to avoid getting Cannot assign to read only property 'stackTraceLimit' of function 'function Error() { [native code] }' errors thrown from core when Node.js is executed with the --frozen-intrinsics flag.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/37405/

[bmeck]

Realized this should be checking configurable, not frozen status to handle if someone does an Object.seal(Error), etc.

[aduh95]

Benchmark CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/998/

Results look OK-ish:

                                                               confidence improvement accuracy (*)    (**)   (***)
misc/hidestackframes.js n=100000 type='direct-call-noerr'                      0.02 %       ±5.94%  ±7.91% ±10.30%
misc/hidestackframes.js n=100000 type='direct-call-throw'               *     -2.86 %       ±2.36%  ±3.14%  ±4.09%
misc/hidestackframes.js n=100000 type='hide-stackframes-noerr'                 0.34 %      ±15.71% ±20.91% ±27.25%
misc/hidestackframes.js n=100000 type='hide-stackframes-throw'                -2.86 %       ±3.23%  ±4.30%  ±5.60%

[aduh95]

We could try to cache the value when the property is not configurable:

Suggested change

	return desc.writable ?? typeof desc.set === 'function';
	const returnValue = desc.writable ?? typeof desc.set === 'function';
	if (desc.configurable === false)
	module.exports.isErrorStackTraceLimitWritable = () => returnValue;
	return returnValue;

I think it makes the harder to read, and I think we should try to optimize for the most common case (which is when the property has configurable and writable set to true).

[bmeck]

I'm fine not cacheing for now

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/37440/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/37441/

[aduh95]

Benchmark CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/999/

EDIT: the results are not as good… I'd say the tradeoff is worth it, I'm open to suggestions to improve those figures.

                                                               confidence improvement accuracy (*)    (**)   (***)
misc/hidestackframes.js n=100000 type='direct-call-noerr'                      2.68 %       ±4.40%  ±5.86%  ±7.62%
misc/hidestackframes.js n=100000 type='direct-call-throw'              **     -4.20 %       ±3.04%  ±4.05%  ±5.29%
misc/hidestackframes.js n=100000 type='hide-stackframes-noerr'                13.07 %      ±14.03% ±18.86% ±24.95%
misc/hidestackframes.js n=100000 type='hide-stackframes-throw'        ***     -4.70 %       ±2.42%  ±3.22%  ±4.19%

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/37457/

[aduh95]

I've added more tests. I think this is ready to land but I'd love to get a final approval to make sure I haven't forgotten anything :)

[bmeck]

LGTM

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/37465/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/37472/

[aduh95]

Landed in 09c9e5d