Skip to content
New issue

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

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

Already on GitHub? # to your account

refactor: call transformError when error is thrown outside of actions #611

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lionelhorn
Copy link
Contributor

Summary of changes

  • Previously transformError was not called when an error occured in kernel.authenticate() for instance
  • Enable better sanitization of errors

Relinquishment of Rights

Please mark following checkbox to confirm that you relinquish all rights of your changes:

  • I waive and relinquish all rights regarding this changes (including code, text, and images) to Deepkit UG (limited), Germany. This changes (including code, text, and images) are under MIT license without name attribution, copyright notice, and permission notice requirement.

- Previously transformError was not called when an error occured in kernel.authenticate() for instance
- Enable better sanitization of errors
if(error.stack === "") {
e.stack = ""
}

Copy link
Member

Choose a reason for hiding this comment

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

this does not include errors that are nominally thrown like these (since it early exits at line 730)

@entity.name('@error:user')
class UserNotFound extends Error{}

while looking at that code I really wonder if we get then even a good stack trace at all even within the client. it seems the new e.stack = '' and the old e.stack = error.stack ... just drops all the information, which is not good. we likely want to maintain the actual stack trace on the client side so users see in what code the error was thrown

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

Successfully merging this pull request may close these issues.

2 participants