Skip to content

feat: add captureStackTrace parameter#145

Merged
gurgunday merged 4 commits intomainfrom
stack
Mar 12, 2025
Merged

feat: add captureStackTrace parameter#145
gurgunday merged 4 commits intomainfrom
stack

Conversation

@gurgunday
Copy link
Member

From fastify/fastify#5925

This is to gather feedback

It allows us to have a centralized way to disable stack traces for Fastify errors

The global createError.captureStackTrace property can be used to disable stack generation for FastifyErrors when the parameter is not specified

@gurgunday gurgunday requested review from a team, Uzlopak and climba03003 March 12, 2025 01:05
@Fdawgs
Copy link
Member

Fdawgs commented Mar 12, 2025

Needs a unit test?


```
createError(code, message [, statusCode [, Base]])
createError(code, message [, statusCode [, Base [, captureStackTrace]]])
Copy link
Member

Choose a reason for hiding this comment

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

This is a great example of why I think
#117 (comment) is needed.

Copy link
Member Author

@gurgunday gurgunday Mar 12, 2025

Choose a reason for hiding this comment

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

I agree that at this point it's too many parameters

I was thinking of using this approach to cut a non-breaking release and then changing the signature as a major

That way we can have some time to refactor other codebases

Copy link
Member

Choose a reason for hiding this comment

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

I agree to make it as two step, release as feature then refactor the params.

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@gurgunday gurgunday merged commit f87dc81 into main Mar 12, 2025
13 checks passed
@gurgunday gurgunday deleted the stack branch March 12, 2025 18:33
@gurgunday
Copy link
Member Author

I'll cut a release

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.

6 participants