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

Add Exception Handling Around More Recursive Extractor Calls #599

Merged
merged 3 commits into from
Jan 10, 2025
Merged

Conversation

gfs
Copy link
Contributor

@gfs gfs commented Jan 8, 2025

  • Try to fix Analysis crash with --no-show-progress option. #598. I wasn't able to reproduce the exact exception thrown in the report, but did identify cases where the calls now wrapped in a try/catch block could throw and stop enumeration and believe this may be the root cause.
  • Also adds better handling for edge case where index is beyond end of file.
  • No-op cleanup of capitalization of a variable name.

gfs added 3 commits January 8, 2025 11:09
Was able to reproduce overflow exceptions thrown here causing end to execution, I think this is potentially the same root cause of issue #598's report of null streams causing a crash as well.
Copy link
Contributor

@danfiedler-msft danfiedler-msft left a comment

Choose a reason for hiding this comment

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

LGTM. I believe your changes will resolve the associated issue.

Optional: this is an opportunity to add some simple tests for TextContainer and add a "no exception thrown"-style test for AnalyzeCommand.

@gfs gfs merged commit 567ac2f into main Jan 10, 2025
11 checks passed
@gfs gfs deleted the gfs/#598 branch January 10, 2025 22:54
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.

Analysis crash with --no-show-progress option.
2 participants