Raise on docs tarball unpack failures

lib/hexdocs/bucket.ex

@@ -167,25 +167,22 @@ defmodule Hexdocs.Bucket do
 
         case Hexdocs.Store.get_to_file(:repo_bucket, key, tarball_path) do
           :ok ->
-            case Hexdocs.Tar.unpack_to_dir({:file, tarball_path},
-                   repository: repository,
-                   package: package,
-                   version: version
-                 ) do
-              {:ok, dir, files} ->
-                upload_files =
-                  list_upload_files(repository, package, new_latest_version, dir, files, :both)
-
-                paths = MapSet.new(upload_files, &elem(&1, 0))
-                update_versions = [version, new_latest_version]
-
-                upload_new_files(upload_files)
-                delete_old_docs(repository, package, update_versions, paths, :both)
-                purge_hexdocs_cache(repository, package, update_versions, :both)
-
-              {:error, reason} ->
-                Logger.error("Failed unpack #{repository}/#{package} #{version}: #{reason}")
-            end
+            {dir, files} =
+              Hexdocs.Tar.unpack_to_dir!({:file, tarball_path},
+                repository: repository,
+                package: package,
+                version: new_latest_version
+              )
+
+            upload_files =
+              list_upload_files(repository, package, new_latest_version, dir, files, :both)
+
+            paths = MapSet.new(upload_files, &elem(&1, 0))
+            update_versions = [version, new_latest_version]
+
+            upload_new_files(upload_files)
+            delete_old_docs(repository, package, update_versions, paths, :both)
+            purge_hexdocs_cache(repository, package, update_versions, :both)
 
           nil ->
             Logger.error("Failed to get tarball #{repository}/#{package} #{new_latest_version}")

lib/hexdocs/queue.ex

@@ -66,19 +66,16 @@ defmodule Hexdocs.Queue do
 
           case Hexdocs.Store.get_to_file(:repo_bucket, key, tarball_path) do
             :ok ->
-              case Hexdocs.Tar.unpack_to_dir({:file, tarball_path},
-                     repository: repository,
-                     package: package,
-                     version: version
-                   ) do
-                {:ok, _dir, files} ->
-                  update_index_sitemap(repository, key)
-                  update_package_sitemap(repository, key, package, files)
-                  Logger.info("#{key}: done")
-
-                {:error, reason} ->
-                  Logger.error("Failed unpack #{repository}/#{package} #{version}: #{reason}")
-              end
+              {_dir, files} =
+                Hexdocs.Tar.unpack_to_dir!({:file, tarball_path},
+                  repository: repository,
+                  package: package,
+                  version: version
+                )
+
+              update_index_sitemap(repository, key)
+              update_package_sitemap(repository, key, package, files)
+              Logger.info("#{key}: done")
 
             nil ->
               Logger.error("#{key}: package not found in store")
@@ -163,36 +160,33 @@ defmodule Hexdocs.Queue do
         {version, all_versions, retired_versions}
       end
 
-    case Hexdocs.Tar.unpack_to_dir(input,
-           repository: repository,
-           package: package,
-           version: version
-         ) do
-      {:ok, dir, files} ->
-        rewrite_files(dir, files)
-
-        Hexdocs.Bucket.upload(
-          repository,
-          package,
-          version,
-          all_versions,
-          retired_versions,
-          dir,
-          files
-        )
+    {dir, files} =
+      Hexdocs.Tar.unpack_to_dir!(input,
+        repository: repository,
+        package: package,
+        version: version
+      )
 
-        if Hexdocs.Utils.latest_version?(package, version, all_versions) do
-          update_index_sitemap(repository, key)
-          update_package_sitemap(repository, key, package, files)
-          update_package_names_csv(repository)
-        end
+    rewrite_files(dir, files)
 
-        elapsed = System.os_time(:millisecond) - start
-        Logger.info("FINISHED UPLOADING DOCS #{key} #{elapsed}ms")
+    Hexdocs.Bucket.upload(
+      repository,
+      package,
+      version,
+      all_versions,
+      retired_versions,
+      dir,
+      files
+    )
 
-      {:error, reason} ->
-        Logger.error("Failed unpack #{repository}/#{package} #{version}: #{reason}")
+    if Hexdocs.Utils.latest_version?(package, version, all_versions) do
+      update_index_sitemap(repository, key)
+      update_package_sitemap(repository, key, package, files)
+      update_package_names_csv(repository)
     end
+
+    elapsed = System.os_time(:millisecond) - start
+    Logger.info("FINISHED UPLOADING DOCS #{key} #{elapsed}ms")
   end
 
   defp process_search(key, repository, package, version, input, start) do
@@ -205,15 +199,16 @@ defmodule Hexdocs.Queue do
           :error when package in @special_package_names -> version
         end
 
-      case Hexdocs.Tar.unpack_to_dir(input, package: package, version: version) do
-        {:ok, dir, files} ->
-          update_search_index(key, package, version, dir, files)
-          elapsed = System.os_time(:millisecond) - start
-          Logger.info("FINISHED INDEXING DOCS #{key} #{elapsed}ms")
+      {dir, files} =
+        Hexdocs.Tar.unpack_to_dir!(input,
+          repository: repository,
+          package: package,
+          version: version
+        )
 
-        {:error, reason} ->
-          Logger.error("Failed unpack #{package} #{version}: #{reason}")
-      end
+      update_search_index(key, package, version, dir, files)
+      elapsed = System.os_time(:millisecond) - start
+      Logger.info("FINISHED INDEXING DOCS #{key} #{elapsed}ms")
     end
   end
 

lib/hexdocs/tar.ex

@@ -1,13 +1,22 @@
 defmodule Hexdocs.Tar do
   require Logger
 
+  defmodule UnpackError do
+    defexception [:repository, :package, :version, :reason]
+
+    @impl true
+    def message(%{repository: repository, package: package, version: version, reason: reason}) do
+      "Failed to unpack #{repository}/#{package} #{version}: #{reason}"
+    end
+  end
+
   def create(files) do
     files = for {path, contents} <- files, do: {String.to_charlist(path), contents}
     {:ok, tarball} = :hex_tarball.create_docs(files)
     tarball
   end
 
-  def unpack_to_dir({:file, path}, opts \\ []) do
+  def unpack_to_dir!({:file, path}, opts \\ []) do
     repository = Keyword.get(opts, :repository, "UNKNOWN")
     package = Keyword.get(opts, :package, "UNKNOWN")
     version = Keyword.get(opts, :version, "UNKNOWN")
@@ -26,14 +35,15 @@ defmodule Hexdocs.Tar do
           |> Enum.map(&Path.relative_to(&1, output_dir))
 
         files = fix_paths(repository, package, version, files)
-
-        case check_version_dirs(files) do
-          :ok -> {:ok, output_dir, files}
-          {:error, _} = error -> error
-        end
+        check_version_dirs!(repository, package, version, files)
+        {output_dir, files}
 
       {:error, reason} ->
-        {:error, inspect(reason)}
+        raise UnpackError,
+          repository: repository,
+          package: package,
+          version: version,
+          reason: inspect(reason)
     end
   end
 
@@ -55,17 +65,19 @@ defmodule Hexdocs.Tar do
     end)
   end
 
-  defp check_version_dirs(files) do
-    result =
+  defp check_version_dirs!(repository, package, version, files) do
+    ok? =
       Enum.all?(files, fn path ->
         first = Path.split(path) |> hd()
         Version.parse(first) == :error
       end)
 
-    if result do
-      :ok
-    else
-      {:error, "root file or directory name not allowed to match a semver version"}
+    if not ok? do
+      raise UnpackError,
+        repository: repository,
+        package: package,
+        version: version,
+        reason: "root file or directory name not allowed to match a semver version"
     end
   end
 

test/hexdocs/tar_test.exs

@@ -2,12 +2,12 @@ defmodule Hexdocs.TarTest do
   use ExUnit.Case, async: true
   alias Hexdocs.Tar
 
-  test "unpack_to_dir" do
+  test "unpack_to_dir!" do
     blob = Tar.create([{"index.html", "contents"}, {"foo.bar", "contents"}])
     path = Hexdocs.TmpDir.tmp_file("test-tarball")
     File.write!(path, blob)
 
-    assert {:ok, dir, files} = Tar.unpack_to_dir({:file, path})
+    assert {dir, files} = Tar.unpack_to_dir!({:file, path})
     assert File.dir?(dir)
     assert length(files) == 2
     assert "index.html" in files
@@ -19,19 +19,59 @@ defmodule Hexdocs.TarTest do
   test "invalid gzip" do
     path = Hexdocs.TmpDir.tmp_file("test-tarball")
     File.write!(path, "")
-    assert {:error, _} = Tar.unpack_to_dir({:file, path})
+
+    assert_raise Tar.UnpackError, ~r/Failed to unpack hexpm\/foo 1\.0\.0:/, fn ->
+      Tar.unpack_to_dir!({:file, path}, repository: "hexpm", package: "foo", version: "1.0.0")
+    end
   end
 
   test "do not allow root files/directories with version names" do
-    reason = "root file or directory name not allowed to match a semver version"
-
     blob = Tar.create([{"1.0.0", "contents"}])
     path = Hexdocs.TmpDir.tmp_file("test-tarball")
     File.write!(path, blob)
-    assert Tar.unpack_to_dir({:file, path}) == {:error, reason}
+
+    assert_raise Tar.UnpackError,
+                 ~r/root file or directory name not allowed to match a semver version/,
+                 fn ->
+                   Tar.unpack_to_dir!({:file, path},
+                     repository: "hexpm",
+                     package: "foo",
+                     version: "1.0.0"
+                   )
+                 end
 
     blob = Tar.create([{"1.0.0/index.html", "contents"}])
     File.write!(path, blob)
-    assert Tar.unpack_to_dir({:file, path}) == {:error, reason}
+
+    assert_raise Tar.UnpackError,
+                 ~r/root file or directory name not allowed to match a semver version/,
+                 fn ->
+                   Tar.unpack_to_dir!({:file, path},
+                     repository: "hexpm",
+                     package: "foo",
+                     version: "1.0.0"
+                   )
+                 end
+  end
+
+  test "raises on tarball with duplicate mode-0 entries" do
+    path = Hexdocs.TmpDir.tmp_file("test-tarball")
+    {:ok, tar} = :hex_erl_tar.open(String.to_charlist(path), [:write, :compressed])
+
+    for _ <- 1..3 do
+      :ok = :hex_erl_tar.add(tar, "contents", ~c"#", [{:mode, 0}])
+    end
+
+    :ok = :hex_erl_tar.close(tar)
+
+    assert_raise Tar.UnpackError,
+                 ~r/Failed to unpack hexpm\/lustre 5\.7\.0: :eacces/,
+                 fn ->
+                   Tar.unpack_to_dir!({:file, path},
+                     repository: "hexpm",
+                     package: "lustre",
+                     version: "5.7.0"
+                   )
+                 end
   end
 end