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

CORE-208: include stack traces in InvalidPfbExceptions #82

Merged
merged 1 commit into from
Dec 4, 2024

Conversation

davidangb
Copy link
Collaborator

When reading the nodes of a PFB, we may throw InvalidPfbException. When we threw that, we omitted the stack trace. This makes debugging inside cWDS pretty hard; we can't see the underlying error that caused the problem.

This is a small PR to include the underlying cause in those exceptions.

@davidangb davidangb requested a review from a team as a code owner December 4, 2024 21:03
Copy link

sonarqubecloud bot commented Dec 4, 2024

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Benchmark

Benchmark suite Current: 3643595 Previous: 68dcbd4 Ratio
bio.terra.pfb.LibraryBenchmarks.showNodesMedium 1933.7613395079065 ops/s 1859.7220972334542 ops/s 1.04
bio.terra.pfb.LibraryBenchmarks.showNodesSmall 24861.837612940726 ops/s 24782.004073882654 ops/s 1.00
bio.terra.pfb.PfbReaderBenchmarks.convertEnum 5260558.121848094 ops/s 5256141.2115673125 ops/s 1.00

This comment was automatically generated by workflow using github-action-benchmark.

Copy link
Contributor

@calypsomatic calypsomatic left a comment

Choose a reason for hiding this comment

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

Did we have good security reasons for omitting the stack trace?

@davidangb
Copy link
Collaborator Author

Did we have good security reasons for omitting the stack trace?

We shouldn't bubble up a stack trace to the user. But, that'll be WDS's job to scrub; here in the library we're not doing anything user-facing. We didn't even give WDS the option to hide or display the stack trace!

@davidangb davidangb merged commit a9169fc into main Dec 4, 2024
11 checks passed
@davidangb davidangb deleted the da_CORE-208_exceptionDetails branch December 4, 2024 21:10
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.

3 participants