[Backport 5.3.9104] scip-ctags: better error propagation (#61847)

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 47ab5f5828)

Co-authored-by: Julie Tibshirani <julietibs@apache.org>

docker-images/syntax-highlighter/crates/syntax-analysis/src/ctags.rs

@@ -197,16 +197,12 @@ pub fn generate_tags<W: std::io::Write>(
     let (root_scope, _) = match get_globals(parser, file_data) {
         Ok(vals) => vals,
         Err(err) => {
-            // TODO: Not sure I want to keep this or not
-            #[cfg(debug_assertions)]
-            if true {
-                panic!("Could not parse file: {}", err);
-            }
-
-            let _ = err;
-
-            return None;
-        }
+            Reply::Error {
+                message: err.to_string(),
+                fatal: false,
+            }.write(buf_writer);
+            return None
+        },
     };
 
     let mut scope_deduplicator = HashMap::new();
@@ -242,15 +238,7 @@ pub fn ctags_runner<R: Read, W: Write>(
             break;
         }
 
-        let request = serde_json::from_str::<Request>(&line);
-        let request = match request {
-            Ok(request) => request,
-            Err(_) => {
-                eprintln!("Could not parse request: {}", line);
-                continue;
-            }
-        };
-
+        let request = serde_json::from_str::<Request>(&line)?;
         match request {
             Request::GenerateTags { filename, size } => {
                 let mut file_data = vec![0; size];
@@ -258,15 +246,21 @@ pub fn ctags_runner<R: Read, W: Write>(
                     .read_exact(&mut file_data)
                     .expect("Could not fill file data exactly");
 
-                let code = String::from_utf8(file_data).context("when generating tags")?;
-                generate_tags(output, filename, &code);
+                match String::from_utf8(file_data) {
+                    Ok(code) => {
+                        generate_tags(output, filename, &code);
+                    }
+                    Err(error) => Reply::Error {
+                        message: error.to_string(),
+                        fatal: false,
+                    }.write(output)
+                };
             }
         }
 
         Reply::Completed {
             command: "generate-tags".to_string(),
-        }
-        .write(output);
+        }.write(output);
 
         output.flush().unwrap();
     }

docker-images/syntax-highlighter/crates/syntax-analysis/src/lib.rs

@@ -117,4 +117,7 @@ mod test {
         test_scip_tags_go_constant,
         "go-const.go"
     );
+
+    // Test that errors are returned in expected format
+    generate_tags_and_snapshot!(Tags, test_tags_perl_example, "example.pl");
 }

docker-images/syntax-highlighter/crates/syntax-analysis/src/snapshots/syntax_analysis__test__tags_snapshot_example.pl.snap

@@ -0,0 +1,6 @@
+---
+source: crates/syntax-analysis/src/lib.rs
+expression: "String::from_utf8_lossy(buf_writer.buffer())"
+---
+{"_type":"error","message":"No tag configuration for language: Perl","fatal":false}
+

docker-images/syntax-highlighter/crates/syntax-analysis/testdata/example.pl

@@ -0,0 +1,3 @@
+#!/usr/bin/perl
+
+print("Hello World\n");

docker-images/syntax-highlighter/src/bin/scip-ctags.rs

@@ -1,6 +1,6 @@
-use std::io::{BufReader, BufWriter};
+use std::io::{BufReader, BufWriter, Write};
 
-use syntax_analysis::ctags::ctags_runner;
+use syntax_analysis::ctags::{ctags_runner, Reply};
 
 fn main() {
     // Exits with a code zero if the environment variable SANITY_CHECK equals
@@ -20,6 +20,12 @@ fn main() {
     let mut stdout = BufWriter::new(std::io::stdout());
 
     if let Err(err) = ctags_runner(&mut stdin, &mut stdout) {
-        eprintln!("Error while executing: {}", err);
+        // If there's a top-level error, treat it as fatal. If an error is recoverable,
+        // we would've already handled it and included it in the ctags response.
+        Reply::Error {
+            message: err.to_string(),
+            fatal: true,
+        }.write(&mut stdout);
+        stdout.flush().unwrap()
     }
 }