Skip to content
This repository has been archived by the owner on Sep 14, 2021. It is now read-only.

Commit

Permalink
Continue serving requests when highlighting an individual file fails (#…
Browse files Browse the repository at this point in the history
…20)

When Syntect fails to highlight a file it often does so by panicking. Most
often these issues come from us making use of syntax definitions which simply
aren't supported by Syntect very well (e.g. they use some new or obscure ST3
feature). There is an upstream issue to [return a `Result` type instead of
panicking](trishume/syntect#98) when this occurs.

Prior to this change, a user requesting syntect_server to highlight a bad file
(i.e. hitting a case in the syntax definition not supported by Syntect) would
result in `syntect_server` dying. This has been a known issue for a while, but
in practice hasn't been that bad because these cases are relatively rare and
Kubernetes / Docker restarts the process very quickly anyway. However, when it
does occur it terminates all ongoing highlighting requests which causes blips
that users see.

After this change, we handle these panics by catching and unwinding the stack.
This isn't perfect / ideal / idiomatic Rust code (see the `catch_unwind` docs),
but it does solve the problem and is a better approach than e.g. adding more
replicas of this service.

Fixes sourcegraph/sourcegraph#3164
  • Loading branch information
slimsag committed Apr 19, 2019
1 parent 056c730 commit 5e1efbb
Showing 1 changed file with 15 additions and 1 deletion.
16 changes: 15 additions & 1 deletion src/main.rs
Expand Up @@ -13,7 +13,7 @@ use std::env;
use std::path::Path;
use syntect::highlighting::ThemeSet;
use syntect::parsing::SyntaxSet;

use std::panic;
use std::fmt::Write;
use syntect::easy::HighlightLines;
use syntect::highlighting::{Color, Theme};
Expand Down Expand Up @@ -45,6 +45,20 @@ struct Query {

#[post("/", format = "application/json", data = "<q>")]
fn index(q: Json<Query>) -> JsonValue {
// TODO(slimsag): In an ideal world we wouldn't be relying on catch_unwind
// and instead Syntect would return Result types when failures occur. This
// will require some non-trivial work upstream:
// https://github.com/trishume/syntect/issues/98
let result = panic::catch_unwind(|| {
highlight(q)
});
match result {
Ok(v) => v,
Err(_) => json!({"error": "panic while highlighting code", "code": "panic"}),
}
}

fn highlight(q: Json<Query>) -> JsonValue {
SYNTAX_SET.with(|syntax_set| {
// Determine theme to use.
//
Expand Down

0 comments on commit 5e1efbb

Please sign in to comment.