diff --git a/server/src/at_record_server/handlers/crate_overlap.gleam b/server/src/at_record_server/handlers/crate_overlap.gleam index 949c157..d0e2a5a 100644 --- a/server/src/at_record_server/handlers/crate_overlap.gleam +++ b/server/src/at_record_server/handlers/crate_overlap.gleam @@ -48,6 +48,8 @@ fn do_get_crate_overlap( case shelf_owner.fetch_public_entries(ctx, pds, did) { Error(shelf_owner.FetchFailed) -> error_json(502, "could not load that user's crate from their PDS") + Error(shelf_owner.RepoNotFound) -> + error_json(404, "that user's crate could not be found") Error(shelf_owner.TooManyPages) -> error_json(413, "that user's crate is too large to page through") Ok(their_stored) -> diff --git a/server/src/at_record_server/handlers/shelf.gleam b/server/src/at_record_server/handlers/shelf.gleam index c15279e..ad051ff 100644 --- a/server/src/at_record_server/handlers/shelf.gleam +++ b/server/src/at_record_server/handlers/shelf.gleam @@ -65,6 +65,8 @@ fn list_actor_shelf(req: Request, ctx: Context, actor: String) -> Response { case shelf_owner.fetch_public_entries(ctx, pds, did) { Error(shelf_owner.FetchFailed) -> error_json(502, "could not load that user's crate from their PDS") + Error(shelf_owner.RepoNotFound) -> + error_json(404, "that user's crate could not be found") Error(shelf_owner.TooManyPages) -> error_json(413, "that user's crate is too large to page through") Ok(events) -> shelf_response(req, ctx, did, handle, events, True) @@ -250,6 +252,8 @@ fn actor_entry(ctx: Context, actor: String, entry_id: String) -> Response { case shelf_owner.fetch_public_entries(ctx, pds, did) { Error(shelf_owner.FetchFailed) -> error_json(502, "could not load that user's crate from their PDS") + Error(shelf_owner.RepoNotFound) -> + error_json(404, "that user's crate could not be found") Error(shelf_owner.TooManyPages) -> error_json(413, "that user's crate is too large to page through") Ok(stored) -> entry_response(ctx, did, handle, stored, entry_id) diff --git a/server/src/at_record_server/shelf_owner.gleam b/server/src/at_record_server/shelf_owner.gleam index 0a8347b..7d4bff5 100644 --- a/server/src/at_record_server/shelf_owner.gleam +++ b/server/src/at_record_server/shelf_owner.gleam @@ -10,6 +10,7 @@ import at_record/gen/repo/list_records.{type RecordEntry} import at_record/gen/shelf/entry import at_record/storage.{type StoredItem} import at_record_server/context.{type Context} +import atproto/xrpc import gleam/dynamic/decode import gleam/int import gleam/list @@ -41,7 +42,7 @@ pub fn resolve_actor( "shelf_owner: resolver unreachable for " <> actor <> ": " - <> string.inspect(err), + <> xrpc.describe(err), ) Error(Nil) } @@ -112,11 +113,13 @@ fn is_private_ipv4(octets: List(Int)) -> Bool { } } -/// Hitting the page cap is a distinct outcome from a transport/decode -/// failure, so the handler can answer honestly instead of handing back a -/// silently short list. +/// Three outcomes the handler must tell apart: a genuine transport/decode +/// failure (`FetchFailed`, 502-worthy), a foreign PDS answering 400/404 +/// because the repo is missing or gone (`RepoNotFound`, 404-worthy, not the +/// caller's fault), and hitting the page cap (`TooManyPages`, neither). pub type FetchEntriesError { FetchFailed + RepoNotFound TooManyPages } @@ -166,7 +169,40 @@ fn fetch_pages( None, ) { - Error(_) -> Error(FetchFailed) + Error(generated_client.RepoListRecordsTransport(err)) -> { + wisp.log_warning( + "shelf_owner: transport failure fetching entries for " + <> did + <> ": " + <> xrpc.describe(err), + ) + Error(FetchFailed) + } + Error(generated_client.RepoListRecordsUnexpected(status, message, _)) -> + case status { + 400 | 404 -> { + wisp.log_warning( + "shelf_owner: no such repo for " + <> did + <> " (pds status " + <> int.to_string(status) + <> ")", + ) + Error(RepoNotFound) + } + _ -> { + wisp.log_warning( + "shelf_owner: unexpected pds status fetching entries for " + <> did + <> ": " + <> int.to_string(status) + <> { + option.map(message, fn(m) { " " <> m }) |> option.unwrap("") + }, + ) + Error(FetchFailed) + } + } Ok(output) -> { let rows = decode_rows(did, output.records) let all = list.append(acc, rows) diff --git a/server/test/actor_shelf_test.gleam b/server/test/actor_shelf_test.gleam index e6dd464..f89a475 100644 --- a/server/test/actor_shelf_test.gleam +++ b/server/test/actor_shelf_test.gleam @@ -142,6 +142,21 @@ fn pds_unreachable_client() -> xrpc.Client { }) } +// The foreign PDS is reachable but says the repo is missing, distinct from +// `pds_unreachable_client`'s transport failure: this should read as 404, +// not 502. +fn pds_repo_not_found_client() -> xrpc.Client { + xrpc.Client(send: fn(req) { + case req.host { + "resolver.test" -> + Ok(response.Response(200, [], bit_array.from_string(resolve_body()))) + host if host == pds_host -> + Ok(response.Response(404, [], bit_array.from_string("{}"))) + _ -> panic as "unexpected host" + } + }) +} + fn a_release() -> catalog_release.CatalogRelease { catalog_release.CatalogRelease( ..support.blank_catalog_release(), @@ -277,12 +292,24 @@ pub fn error_status_test() { support.xrpc("shelf.listEntries?actor=pub.test"), 502, ), + #( + shelf.list_shelf, + pds_repo_not_found_client(), + support.xrpc("shelf.listEntries?actor=pub.test"), + 404, + ), #( shelf.get_entry, network_client(known_entry), support.xrpc("shelf.getEntry?actor=pub.test&entry=missing"), 404, ), + #( + shelf.get_entry, + pds_repo_not_found_client(), + support.xrpc("shelf.getEntry?actor=pub.test&entry=3aaa"), + 404, + ), #( shelf.get_entry, failing_resolver_client(),