This repository was archived by the owner on Sep 30, 2024. It is now read-only.
scip-ctags: better error propagation#61712
Merged
Merged
Conversation
jtibshirani
commented
Apr 9, 2024
| Ok(vals) => vals, | ||
| Err(err) => { | ||
| // TODO: Not sure I want to keep this or not | ||
| #[cfg(debug_assertions)] |
Contributor
Author
There was a problem hiding this comment.
I removed this since I think we should always surface errors instead of silently squashing them.
Contributor
Author
|
Highlighting that I chose to make "unsupported language" a non-fatal error. In terms of user impact, this means we won't fail indexing an entire Zoekt repo if we can't parse a single file due to its language. Background: this happens sometimes because we can misroute files. Zoekt performs precise language detection, so it classifies iamjs.pl as Javascript and passes it to scip-ctags. However, scip-ctags thinks this is Perl, which it can't handle. (We should fix this too, but it's been a problem for a while, both with universal-ctags and scip-ctags!) Note: as a follow-up, we should improve Zoekt error alerting to clearly surface when we fail to parse a file, so we can iteratively fix these issues. |
varungandhi-src
suggested changes
Apr 10, 2024
varungandhi-src
approved these changes
Apr 10, 2024
sourcegraph-release-bot
pushed a commit
that referenced
this pull request
Apr 12, 2024
This change corrects the error handling in scip-ctags, making it more consistent with universal-ctags. We now split errors into two classes: * Fatal: user command is invalid or incorrectly formatted, I/O errors. This indicates a big issue in how we're using ctags and we should fail hard. * Non-fatal: any file parsing issue (invalid UTF-8, unsupported language, or all symbols can't be parsed). Our go-ctags wrapper only reports fatal errors, and just logs non-fatal errors. We fail indexing for an entire Zoekt repo on go-ctags errors, which helps surface serious issues with ctags, while allowing parse failures for individual files. (cherry picked from commit 47ab5f5)
BolajiOlajide
pushed a commit
that referenced
this pull request
Apr 15, 2024
scip-ctags: better error propagation (#61712) This change corrects the error handling in scip-ctags, making it more consistent with universal-ctags. We now split errors into two classes: * Fatal: user command is invalid or incorrectly formatted, I/O errors. This indicates a big issue in how we're using ctags and we should fail hard. * Non-fatal: any file parsing issue (invalid UTF-8, unsupported language, or all symbols can't be parsed). Our go-ctags wrapper only reports fatal errors, and just logs non-fatal errors. We fail indexing for an entire Zoekt repo on go-ctags errors, which helps surface serious issues with ctags, while allowing parse failures for individual files. (cherry picked from commit 47ab5f5) Co-authored-by: Julie Tibshirani <julietibs@apache.org>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change corrects the error handling in scip-ctags, making it more
consistent with universal-ctags. We now split errors into two classes:
indicates a big issue in how we're using ctags and we should fail hard.
can't be parsed).
Our go-ctags wrapper only reports fatal errors, and just logs non-fatal errors.
We fail indexing for an entire Zoekt repo on go-ctags errors, which helps
surface serious issues with ctags, while allowing parse failures for individual files.
Fixes https://github.com/sourcegraph/sourcegraph/issues/61785
Fixes ENG-23595
Test plan
Added a new snapshot test for an error case, and made sure the output looked
good. Manual testing, including indexing the
github-linguist/linguistrepolocally and checking it now succeeds.