Merge pull request #553 from jyn514/delete-the-panic-button

Fix occasional panic when visiting settings.html

Author: pietroalbini

src/db/add_package.rs

@@ -22,11 +22,14 @@ use failure::err_msg;
 /// Adds a package into database.
 ///
 /// Package must be built first.
+///
+/// NOTE: `source_files` refers to the files originally in the crate,
+/// not the files generated by rustdoc.
 pub(crate) fn add_package_into_database(conn: &Connection,
                                  metadata_pkg: &MetadataPackage,
                                  source_dir: &Path,
                                  res: &BuildResult,
-                                 files: Option<Json>,
+                                 source_files: Option<Json>,
                                  doc_targets: Vec<String>,
                                  default_target: &Option<String>,
                                  cratesio_data: &CratesIoData,
@@ -78,7 +81,7 @@ pub(crate) fn add_package_into_database(conn: &Connection,
                                          &metadata_pkg.keywords.to_json(),
                                          &has_examples,
                                          &cratesio_data.downloads,
-                                         &files,
+                                         &source_files,
                                          &doc_targets.to_json(),
                                          &is_library,
                                          &res.rustc_version,
@@ -132,7 +135,7 @@ pub(crate) fn add_package_into_database(conn: &Connection,
                               &metadata_pkg.keywords.to_json(),
                               &has_examples,
                               &cratesio_data.downloads,
-                              &files,
+                              &source_files,
                               &doc_targets.to_json(),
                               &is_library,
                               &res.rustc_version,

src/db/file.rs

@@ -142,7 +142,15 @@ pub(super) fn s3_client() -> Option<S3Client> {
     ))
 }
 
-/// Adds files into database and returns list of files with their mime type in Json
+/// Store all files in a directory and return [[mimetype, filename]] as Json
+///
+/// If there is an S3 Client configured, store files into an S3 bucket;
+/// otherwise, stores files into the 'files' table of the local database.
+///
+/// The mimetype is detected using `magic`.
+///
+/// Note that this function is used for uploading both sources
+/// and files generated by rustdoc.
 pub fn add_path_into_database<P: AsRef<Path>>(conn: &Connection,
                                               prefix: &str,
                                               path: P)

src/test/fakes.rs

@@ -3,22 +3,25 @@ use crate::db::CratesIoData;
 use crate::docbuilder::BuildResult;
 use crate::utils::{Dependency, MetadataPackage, Target};
 use failure::Error;
-use rustc_serialize::json::Json;
 
-pub(crate) struct FakeRelease<'db> {
-    db: &'db TestDatabase,
+#[must_use = "FakeRelease does nothing until you call .create()"]
+pub(crate) struct FakeRelease<'a> {
+    db: &'a TestDatabase,
     package: MetadataPackage,
     build_result: BuildResult,
-    files: Option<Json>,
+    /// name, content
+    source_files: Vec<(&'a str, &'a [u8])>,
+    /// name, content
+    rustdoc_files: Vec<(&'a str, &'a [u8])>,
     doc_targets: Vec<String>,
     default_target: Option<String>,
     cratesio_data: CratesIoData,
     has_docs: bool,
     has_examples: bool,
 }
 
-impl<'db> FakeRelease<'db> {
-    pub(super) fn new(db: &'db TestDatabase) -> Self {
+impl<'a> FakeRelease<'a> {
+    pub(super) fn new(db: &'a TestDatabase) -> Self {
         FakeRelease {
             db,
             package: MetadataPackage {
@@ -46,7 +49,8 @@ impl<'db> FakeRelease<'db> {
                 build_log: "It works!".into(),
                 successful: true,
             },
-            files: None,
+            source_files: Vec::new(),
+            rustdoc_files: Vec::new(),
             doc_targets: Vec::new(),
             default_target: None,
             cratesio_data: CratesIoData {
@@ -81,15 +85,38 @@ impl<'db> FakeRelease<'db> {
         self
     }
 
+    pub(crate) fn rustdoc_file(mut self, path: &'a str, data: &'a [u8]) -> Self {
+        self.rustdoc_files.push((path, data));
+        self
+    }
+
     pub(crate) fn create(self) -> Result<i32, Error> {
         let tempdir = tempdir::TempDir::new("docs.rs-fake")?;
 
+        let upload_files = |prefix: &str, files: &[(&str, &[u8])]| {
+            let path_prefix = tempdir.path().join(prefix);
+            std::fs::create_dir(&path_prefix)?;
+
+            for (path, data) in files {
+                let file = path_prefix.join(&path);
+                std::fs::write(file, data)?;
+            }
+
+            let prefix = format!("{}/{}/{}", prefix, self.package.name, self.package.version);
+            crate::db::add_path_into_database(&self.db.conn(), &prefix, path_prefix)
+        };
+
+        let rustdoc_meta = upload_files("rustdoc", &self.rustdoc_files)?;
+        log::debug!("added rustdoc files {}", rustdoc_meta);
+        let source_meta = upload_files("source", &self.source_files)?;
+        log::debug!("added source files {}", source_meta);
+
         let release_id = crate::db::add_package_into_database(
             &self.db.conn(),
             &self.package,
             tempdir.path(),
             &self.build_result,
-            self.files,
+            Some(source_meta),
             self.doc_targets,
             &self.default_target,
             &self.cratesio_data,

src/test/mod.rs

@@ -6,11 +6,17 @@ use once_cell::unsync::OnceCell;
 use postgres::Connection;
 use reqwest::{Client, Method, RequestBuilder};
 use std::sync::{Arc, Mutex, MutexGuard};
+use std::panic;
 
 pub(crate) fn wrapper(f: impl FnOnce(&TestEnvironment) -> Result<(), Error>) {
     let env = TestEnvironment::new();
-    let result = f(&env);
+    // if we didn't catch the panic, the server would hang forever
+    let maybe_panic = panic::catch_unwind(panic::AssertUnwindSafe(|| f(&env)));
     env.cleanup();
+    let result = match maybe_panic {
+        Ok(r) => r,
+        Err(payload) => panic::resume_unwind(payload),
+    };
 
     if let Err(err) = result {
         eprintln!("the test failed: {}", err);
@@ -24,6 +30,13 @@ pub(crate) fn wrapper(f: impl FnOnce(&TestEnvironment) -> Result<(), Error>) {
     }
 }
 
+/// Make sure that a URL returns a status code between 200-299
+pub(crate) fn assert_success(path: &str, web: &TestFrontend) -> Result<(), Error> {
+    let status = web.get(path).send()?.status();
+    assert!(status.is_success(), "failed to GET {}: {}", path, status);
+    Ok(())
+}
+
 pub(crate) struct TestEnvironment {
     db: OnceCell<TestDatabase>,
     frontend: OnceCell<TestFrontend>,

src/web/rustdoc.rs

@@ -311,8 +311,12 @@ fn path_for_version(req_path: &[&str], target_name: &str, conn: &Connection) ->
             .expect("paths should be of the form <kind>.<name>.html")
     };
     // check if req_path[3] is the platform choice or the name of the crate
+    // rustdoc generates a ../settings.html page, so if req_path[3] is not
+    // the target, that doesn't necessarily mean it's a platform.
+    // we also can't check if it's in TARGETS, since some targets have been
+    // removed (looking at you, i686-apple-darwin)
     let concat_path;
-    let crate_root = if req_path[3] != target_name {
+    let crate_root = if req_path[3] != target_name && req_path.len() >= 5 {
         concat_path = format!("{}/{}", req_path[3], req_path[4]);
         &concat_path
     } else {
@@ -408,3 +412,32 @@ impl Handler for SharedResourceHandler {
         Err(IronError::new(Nope::ResourceNotFound, status::NotFound))
     }
 }
+
+#[cfg(test)]
+mod test {
+    use crate::test::*;
+    #[test]
+    // regression test for https://github.com/rust-lang/docs.rs/issues/552
+    fn settings_html() {
+        wrapper(|env| {
+            let db = env.db();
+            // first release works, second fails
+            db.fake_release()
+              .name("buggy").version("0.1.0")
+              .build_result_successful(true)
+              .rustdoc_file("settings.html", b"some data")
+              .rustdoc_file("all.html", b"some data 2")
+              .create()?;
+            db.fake_release()
+              .name("buggy").version("0.2.0")
+              .build_result_successful(false)
+              .create()?;
+            let web = env.frontend();
+            assert_success("/", web)?;
+            assert_success("/crate/buggy/0.1.0/", web)?;
+            assert_success("/buggy/0.1.0/settings.html", web)?;
+            assert_success("/buggy/0.1.0/all.html", web)?;
+            Ok(())
+        });
+    }
+}