Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance of stack traces in errors results in high response latency (>1 second) #4028

Closed
ajhenry opened this issue Mar 12, 2024 · 3 comments

Comments

@ajhenry
Copy link

ajhenry commented Mar 12, 2024

Great work y'all are doing here! We've run into a bit of a snag dealing with errors and stack traces.

The problem:
As bundle size increases, the duration of calculating a stack trace with source maps of an error thrown during resolver execution takes more time to compute.

We narrowed down the culprit to the following check:

if (originalError?.stack != null) {
Object.defineProperty(this, 'stack', {
value: originalError.stack,
writable: true,
configurable: true,
});
} else if (Error.captureStackTrace != null) {
Error.captureStackTrace(this, GraphQLError);
} else {
Object.defineProperty(this, 'stack', {
value: Error().stack,
writable: true,
configurable: true,
});
}

We did a flame graph for an error thrown and this resulted in the following:

Image
Image

It takes this amount of time regardless of which resolver throws an error.

We currently bundle our server (along with tree-shaken dependencies) in a single file so we can upload it to a lambda. We are at around 76MB for our total bundled file which is not great (😟) but still very usable for us in production.

Our workaround right now is to explicitly set captureStackTrace and bypass having to perform this calculation. This decreases the execution time from 1154ms to 20ms. However, we are left with no stack traces.

Any suggestions or help would be greatly appreciated, thanks!

@yaacovCR
Copy link
Contributor

Duplicate of #2800 ?
That issue suggests disabling source maps?

@JoviDeCroock
Copy link
Member

JoviDeCroock commented Oct 11, 2024

So doing Error().stack which is the fallback when not calling captureStackTrace doesn't take up this much compute? We could just always leverage Error().stack and filter out the GraphQLError line which is essentially all that captureStackTrace should be doing here. Alternatively, our stack will be very deep, we could set a stackTraceLimit which should reduce the performance impact, all though the lookup in the sourcemaps should always be expensive.

The node team does have this as an issue as well nodejs/node#41541 (comment) - in case you are leveraging ESBuild there are tips in there as well. Also a noteworthy PR nodejs/node#43875 - so this performance hit should only be applicable when the error is unhandled.

@JoviDeCroock
Copy link
Member

Going to close this as a duplicate of #2800

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants