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

[Discussion] Intentional to mask error.cause? #123

Closed
wants to merge 1 commit into from

Conversation

KATT
Copy link
Contributor

@KATT KATT commented Feb 7, 2023

Thought it was easier to create a discussion with my proposed change here.

I was debugging some errors in production and noticed that my cause-tree was missing and I think I have drilled it down to this line of code.

Often times, libraries wrap errors in their own errors so they are homogenic (we do this in tRPC by wrapping all in TRPCError with the original error as the cause and Apollo Server does something similar with GraphQL Errors IIRC).

For debugging however, the original error may contain useful properties, which seem to be hidden here.

I can work around this, but wanted to ask some more motivation on how come this is and if this is intentional?

It might be me misinterpreting how to use things

Thought it was easier to create a discussion with my proposed change here.

I was debugging some errors in production and noticed that my `cause`-tree was missing and I think I have drilled it down to this line of code.

Often times, libraries wrap errors in their own errors so they are homogenic (we do this in tRPC by wrapping all in `TRPCError` with the original error as the `cause` and Apollo Server does something similar with GraphQL Errors IIRC). 

For debugging however, the original error may contain useful properties, which seem to be hidden here.

I can work around this, but wanted to ask some more motivation on how come this is and if this is intentional? 

It might be me misinterpreting how to use things
@jsumners
Copy link
Member

jsumners commented Feb 7, 2023

I assume the PR that introduced it has discussion around it. Have you reviewed it?

@KATT
Copy link
Contributor Author

KATT commented Feb 7, 2023

You're right:

I'll open an issue if I actually have a problem.

Sorry for the noise, thanks for such a quick answer.

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.

2 participants