Skip to content

Resolve exception when Error.stackTraceLimit is undefined#2701

Merged
murgatroid99 merged 1 commit intogrpc:masterfrom
davidfiala:patch-1
Apr 1, 2024
Merged

Resolve exception when Error.stackTraceLimit is undefined#2701
murgatroid99 merged 1 commit intogrpc:masterfrom
davidfiala:patch-1

Conversation

@davidfiala
Copy link
Copy Markdown
Contributor

Some applications may explicitly set Error.stackTraceLimit = undefined.

In this case it is not safe to assume new Error().stack is available using the ! operator.

Before this patch, using grpc-js in this situation when an Error occurs or when a trace is needed may result in uncaught exceptions crashing the application.

One thing I considered is that often stack traces are indented by four spaces by the JS engine such as node. My default message for clarity is just no track trace available versus using the magic number of spaces. I'm torn on the right solution here, but I personal prefer the clarity of a plain string rather than assuming our environment is one which adds random spaces to make CLI focused output prettier.

Some applications may explicitly set Error.stackTraceLimit = undefined. In this case it is not safe to assume new Error().stack is available.
@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla bot commented Mar 27, 2024

CLA Signed


The committers listed above are authorized under a signed CLA.

Copy link
Copy Markdown
Member

@murgatroid99 murgatroid99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the fix.

@murgatroid99 murgatroid99 merged commit 74e4da5 into grpc:master Apr 1, 2024
murgatroid99 added a commit that referenced this pull request Apr 1, 2024
…kport

Backport #2701: Resolve exception when Error.stackTraceLimit is undefined
@murgatroid99
Copy link
Copy Markdown
Member

This is out in version 1.10.5.

@davidfiala davidfiala deleted the patch-1 branch April 10, 2024 18:12
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

Successfully merging this pull request may close these issues.

3 participants