Skip to content

Conversation

dxg
Copy link
Contributor

@dxg dxg commented Aug 11, 2025

Description

error.stack being undefined makes debugging very difficult. I lost a few hours the other day trying to pinpoint the source of an error due to the empty stack.
Given this restores standard JS Error behaviour, I have not included any tests.

Resolves #2482


Checklist

  • Does npm test pass with this change (including linting)?
  • Is the new or changed code fully tested?
  • Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?

It was very difficult to debug `ErrorReply` errors due to `error.stack` being erased.
Given this restores standard JS Error behaviour, I have not added any tests.
@nkaradzhov
Copy link
Collaborator

@bobymicroby, @PavelPashov I dont see any reason not to merge this, but just to doublecheck with you?

@bobymicroby
Copy link
Member

@nkaradzhov i don't know what was the rationale behind excluding the stack in first place tbh, but the patch looks ok to me

@nkaradzhov nkaradzhov self-requested a review August 14, 2025 07:40
@nkaradzhov nkaradzhov merged commit 82847fb into redis:master Aug 14, 2025
11 checks passed
@dxg dxg deleted the retain-error-reply-stack branch August 15, 2025 07:33
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.

Why is ErrorReply stack being erased?
3 participants