fix: handle backend responses more gracefully in datasets requests (#46)

Author: mortenpi

CHANGELOG.md

@@ -2,11 +2,17 @@
 
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## Version v0.1.7 - 2024-01-18
+
+### Fixed
+
+* `JuliaHub.datasets` and `JuliaHub.dataset` now handle problematic backend responses more gracefully. (#46)
+
 ## Version v0.1.6 - 2023-11-27
 
 ### Fixed
 
-* `JuliaHub.appbundle`, when it has to generate a `Projec.toml` file, now correctly includes it in the appbundle tarball. (#44)
+* `JuliaHub.appbundle`, when it has to generate a `Project.toml` file, now correctly includes it in the appbundle tarball. (#44)
 * `JuliaHub.appbundle` now works with relative paths such as `"."`. (#44)
 
 ## Version v0.1.5 - 2023-09-27

Project.toml

@@ -1,7 +1,7 @@
 name = "JuliaHub"
 uuid = "bc7fa6ce-b75e-4d60-89ad-56c957190b6e"
 authors = ["JuliaHub Inc."]
-version = "0.1.6"
+version = "0.1.7"
 
 [deps]
 Base64 = "2a0f44e3-6c83-55bd-87e4-b1978d98bd5f"

src/datasets.jl

@@ -312,29 +312,50 @@ function datasets(
     auth::Authentication=__auth__(),
 )
     datasets, _ = try
-        _get_datasets(; auth)
+        _get_datasets(auth)
     catch e
         e isa JuliaHubException && rethrow(e)
         e isa JuliaHubError && rethrow(e)
         throw(
             JuliaHubError("Error while retrieving datasets from the server", e, catch_backtrace()),
         )
     end
-    shared || filter!(datasets) do dataset
-        dataset["owner"]["username"] == username
+    # It might happen that some of the elements of the `datasets` array can not be parsed for some reason,
+    # and the Dataset() constructor will throw. Rather than having `datasets` throw an error (as we would
+    # normally do for invalid backend responses), in this case we handle the situation more gracefully,
+    # and just filter out the invalid elements (and print a warning to the user -- it's still an unusual
+    # situation and should be reported). This is because we also re-use `JuliaHub.datasets` in
+    # `JuliaHub.dataset` when fetching the information for just one dataset, and unconditionally throwing
+    # would mean that `JuliaHub.dataset` can break due an issue with an unrelated dataset.
+    n_erroneous_datasets = 0
+    datasets = map(datasets) do dataset
+        try
+            # We also use the `nothing` method for filtering out datasets that are not owned by the
+            # current `username` if `shared = false`.
+            if !shared && (dataset["owner"]["username"] != username)
+                return nothing
+            end
+            return Dataset(dataset)
+        catch e
+            @debug "Invalid dataset in GET /datasets response" dataset exception = (
+                e, catch_backtrace()
+            )
+            n_erroneous_datasets += 1
+            return nothing
+        end
     end
-    return try
-        Dataset.(datasets)
-    catch e
-        throw(JuliaHubError("Unable to parse the response from the server", e, catch_backtrace()))
+    if n_erroneous_datasets > 0
+        @warn "The JuliaHub GET /datasets response contains erroneous datasets. Omitting $(n_erroneous_datasets) entries."
     end
+    # We'll filter down to just Dataset objects, and enforce type-stability on the array type here.
+    return Dataset[ds for ds in datasets if isa(ds, Dataset)]
 end
 
 function datasets(; auth::Authentication=__auth__(), kwargs...)
     datasets(auth.username; auth, kwargs...)
 end
 
-function _get_datasets(; writable=false, auth)
+function _get_datasets(auth::Authentication; writable=false)
     url_path = writable ? ("user", "datasets") : ("datasets",)
     r = _restcall(auth, :GET, url_path, nothing)
     r.status == 200 || _throw_invalidresponse(r; msg="Unable to fetch datasets.")

test/datasets-live.jl

@@ -83,7 +83,7 @@ try
     JuliaHub.upload_dataset(
         (auth.username, blobname), joinpath(TESTDATA, "hi.txt"); create=false, update=true
     )
-    datasets, _ = JuliaHub._get_datasets(; auth)
+    datasets, _ = JuliaHub._get_datasets(auth)
     blob_dataset_json = only(filter(d -> d["name"] == blobname, datasets))
     @test length(blob_dataset_json["versions"]) == 2
 
@@ -100,7 +100,7 @@ try
     @test tree_dataset.dtype == "BlobTree"
 
     JuliaHub.upload_dataset(treename, TESTDATA; auth, create=false, update=true)
-    datasets, _ = JuliaHub._get_datasets(; auth)
+    datasets, _ = JuliaHub._get_datasets(auth)
     tree_dataset_json = only(filter(d -> d["name"] == treename, datasets))
     @test length(tree_dataset_json["versions"]) == 2
 

test/datasets.jl

@@ -104,6 +104,29 @@ end
             @test ds.description == "An example dataset"
             @test isempty(ds.versions)
         end
+
+        MOCK_JULIAHUB_STATE[:datasets_erroneous] = ["erroneous_dataset"]
+        err_ds_warn = (
+            :warn,
+            "The JuliaHub GET /datasets response contains erroneous datasets. Omitting 1 entries.",
+        )
+        let datasets = @test_nowarn JuliaHub.datasets()
+            @test length(datasets) == 2
+        end
+        let datasets = @test_logs err_ds_warn JuliaHub.datasets(; shared=true)
+            @test length(datasets) == 3
+        end
+        let ds = @test_logs err_ds_warn JuliaHub.dataset("example-dataset")
+            @test ds isa JuliaHub.Dataset
+        end
+        let ds = @test_logs err_ds_warn JuliaHub.dataset("example-dataset")
+            @test ds isa JuliaHub.Dataset
+            @test ds.owner == "username"
+            @test ds.name == "example-dataset"
+        end
+        @test_logs err_ds_warn begin
+            @test_throws JuliaHub.InvalidRequestError JuliaHub.dataset("erroneous_dataset")
+        end
     end
 end
 

test/mocking.jl

@@ -384,6 +384,22 @@ function _restcall_mocked(method, url, headers, payload; query)
                 ),
             )
         end
+        for dataset in get(MOCK_JULIAHUB_STATE, :datasets_erroneous, String[])
+            push!(datasets,
+                Dict(
+                    "id" => string(uuidhash(dataset)),
+                    "name" => dataset,
+                    "owner" => Dict(
+                        "username" => nothing,
+                        "type" => "User",
+                    ),
+                    "type" => occursin("blobtree", dataset) ? "BlobTree" : "Blob",
+                    "visibility" => occursin("public", dataset) ? "public" : "private",
+                    versions_json(dataset)...,
+                    shared...,
+                ),
+            )
+        end
         datasets |> jsonresponse(200)
     elseif (method == :DELETE) && endswith(url, DATASET_REGEX)
         dataset = URIs.unescapeuri(match(DATASET_REGEX, url)[1])