Skip to content

Fix UTF-8 validation and file IO error checking#588

Merged
davidanthoff merged 1 commit into
masterfrom
fix-utf8-validation
Mar 9, 2020
Merged

Fix UTF-8 validation and file IO error checking#588
davidanthoff merged 1 commit into
masterfrom
fix-utf8-validation

Conversation

@davidanthoff

@davidanthoff davidanthoff commented Mar 9, 2020

Copy link
Copy Markdown
Member

When I added the more specific error handlers, I forgot to also adjust the validation phase accordingly. Fixes a bug from crash reporting.

I added some stuff, namely it is now also handling SystemError properly, because those can also happen from file IO problems.

@davidanthoff davidanthoff added this to the Current milestone Mar 9, 2020
@davidanthoff
davidanthoff requested a review from ZacLN March 9, 2020 22:28
@codecov

codecov Bot commented Mar 9, 2020

Copy link
Copy Markdown

Codecov Report

Merging #588 into master will decrease coverage by 0.03%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #588      +/-   ##
==========================================
- Coverage   29.12%   29.08%   -0.04%     
==========================================
  Files          23       23              
  Lines        1686     1688       +2     
==========================================
  Hits          491      491              
- Misses       1195     1197       +2
Impacted Files Coverage Δ
src/requests/textdocument.jl 53.18% <0%> (ø) ⬆️
src/requests/init.jl 17.91% <0%> (ø) ⬆️
src/staticlint.jl 15.62% <0%> (ø) ⬆️
src/requests/workspace.jl 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0be73b7...ddcabee. Read the comment docs.

@ZacLN ZacLN left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could we not re-throw here? This is part of the speculative parent search, if it not a valid file lets just ignore it

I missed this first time around but have just been bitten by it.

Comment thread src/requests/textdocument.jl
@ZacLN

ZacLN commented Mar 9, 2020

Copy link
Copy Markdown
Contributor

Not quite sure if those comments are clear: could we avoid rethrowing the error on lines 408 and 427 of textdocument.jl

@davidanthoff davidanthoff changed the title Fix UTF-8 validation Fix UTF-8 validation and file IO error checking Mar 9, 2020
@davidanthoff

Copy link
Copy Markdown
Member Author

Not quite sure if those comments are clear: could we avoid rethrowing the error on lines 408 and 427 of textdocument.jl

So that is the idea: it will not rethrow for IOError (and now SystemError), but for any other error it will rethrow (under the assumption that that would indicate a problem unrelated to file IO which we want to see in the crash reports).

@ZacLN

ZacLN commented Mar 9, 2020

Copy link
Copy Markdown
Contributor

Ah got you! ignore me..

@davidanthoff

Copy link
Copy Markdown
Member Author

:) can you approve?

@davidanthoff
davidanthoff merged commit f648c00 into master Mar 9, 2020
@davidanthoff
davidanthoff deleted the fix-utf8-validation branch March 9, 2020 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants