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

test(e2e): Add nestjs e2e test documenting errors not being properly caught in submodules #12868

Merged
merged 2 commits into from
Jul 11, 2024

Conversation

nicohrubec
Copy link
Contributor

@nicohrubec nicohrubec commented Jul 11, 2024

Related to this issue.

Currently errors in submodules are not properly handled in nest applications if a custom exception filter extending the BaseExceptionFilter is used, leading to expected exceptions not being caught correctly and therefore being returned as 500 and sent to Sentry.

This expands the sample nest application and adds an e2e test that documents this behavior that can be used to check that future improvements to the SDK actually work. Of course the test has to be adjusted in that case.

@nicohrubec nicohrubec changed the title test(e2e): Add e2e test documenting errors not being properly caught in submodules test(e2e): Add e2e test documenting errors not being properly caught in nestjs submodules Jul 11, 2024
@nicohrubec nicohrubec changed the title test(e2e): Add e2e test documenting errors not being properly caught in nestjs submodules test(e2e): Add nestjs e2e test documenting errors not being properly caught in submodules Jul 11, 2024
Copy link
Member

@chargome chargome left a comment

Choose a reason for hiding this comment

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

l: Calling the module and files "test" could be confusing with actual application testing (like in errors.test.ts). I would rather use "example" or something similar.

@nicohrubec nicohrubec merged commit 0e6d802 into develop Jul 11, 2024
90 checks passed
@nicohrubec nicohrubec deleted the nh/unexpected-error-e2e branch July 11, 2024 08:26
@nicohrubec
Copy link
Contributor Author

@chargome Missed the comment before merging but fair point will rename it in the next PR related to this

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