Skip to content

Commit 42b72ff

Browse files
coryodanielclaude
andcommitted
Standardize error handling to three-tuple map details
User prompts: - "Expand registry error results to include details from storage adapters" - "make the registry do most of the error handling, check for repo/upload existence early" - "come up with a good error struct standard" - "bump the minor level and open a PR" Changes: - Standardize all errors to {:error, atom_code, map()} three-tuple - Add ensure_repo_exists/3 and ensure_upload_exists/4 precondition helpers to Registry - Registry now checks repo/upload existence before delegating to storage - Fix unhandled `false` fall-through in upload_blob_chunk and complete_blob_upload - Convert all string error details to structured maps across auth, storage, handler - Remove two-tuple error match from Handler (and the placeholder message) - Update Storage.Adapter and Auth.Adapter callback specs to enforce map details - Bump version to 0.2.0 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 7b31040 commit 42b72ff

9 files changed

Lines changed: 79 additions & 80 deletions

File tree

lib/oci/auth/adapter.ex

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ defmodule OCI.Auth.Adapter do
3838
"""
3939
@type subject_t :: any()
4040

41-
@type error_details_t :: any()
41+
@type error_details_t :: map()
4242

4343
@callback init(config :: map()) :: {:ok, t()} | {:error, term()}
4444

lib/oci/auth/static.ex

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -45,21 +45,24 @@ defmodule OCI.Auth.Static do
4545
subject = username
4646
{:ok, subject}
4747
else
48-
{:error, :UNAUTHORIZED, "Invalid username or password"}
48+
{:error, :UNAUTHORIZED, %{reason: "Invalid username or password"}}
4949
end
5050

5151
_ ->
5252
{:error, :UNAUTHORIZED,
53-
"Invalid authorization format, should be username:password"}
53+
%{reason: "Invalid authorization format, should be username:password"}}
5454
end
5555

5656
:error ->
5757
{:error, :UNAUTHORIZED,
58-
"Failed to decode authorization, should be base64 encoded username:password"}
58+
%{
59+
reason:
60+
"Failed to decode authorization, should be base64 encoded username:password"
61+
}}
5962
end
6063

6164
_other ->
62-
{:error, :UNSUPPORTED, "Unsupported authentication scheme: #{scheme}"}
65+
{:error, :UNSUPPORTED, %{reason: "Unsupported authentication scheme: #{scheme}"}}
6366
end
6467
end
6568

@@ -73,18 +76,18 @@ defmodule OCI.Auth.Static do
7376

7477
case required_action(ctx.method, ctx.endpoint) do
7578
nil ->
76-
{:error, :DENIED}
79+
{:error, :DENIED, %{repo: ctx.repo, method: ctx.method}}
7780

7881
action ->
7982
if action in repo_perms do
8083
:ok
8184
else
82-
{:error, :DENIED}
85+
{:error, :DENIED, %{repo: ctx.repo, action: action}}
8386
end
8487
end
8588

8689
_ ->
87-
{:error, :DENIED}
90+
{:error, :DENIED, %{subject: ctx.subject}}
8891
end
8992
end
9093

lib/oci/plug.ex

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,9 @@ defmodule OCI.Plug do
3434
end
3535

3636
def call(conn, _opts) do
37-
error_resp(conn, :UNSUPPORTED, "OCI Registry must be mounted at /#{Registry.api_version()}")
37+
error_resp(conn, :UNSUPPORTED, %{
38+
reason: "OCI Registry must be mounted at /#{Registry.api_version()}"
39+
})
3840
end
3941

4042
def authenticate(%{private: %{oci_registry: registry}} = conn) do
@@ -64,16 +66,13 @@ defmodule OCI.Plug do
6466
:ok ->
6567
conn
6668

67-
{:error, reason} ->
68-
error_resp(conn, reason, nil)
69-
7069
{:error, reason, details} ->
7170
error_resp(conn, reason, details)
7271
end
7372
end
7473

7574
defp authorize(_) do
76-
{:error, :UNAUTHORIZED}
75+
{:error, :UNAUTHORIZED, %{}}
7776
end
7877

7978
defp challenge_resp(conn) do

lib/oci/plug/handler.ex

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,6 @@ defmodule OCI.Plug.Handler do
3030
%Plug.Conn{} = conn ->
3131
conn
3232

33-
{:error, oci_error_status} ->
34-
you_suck_and_are_a_bad_person_messsage = """
35-
You suck and are a bad person.
36-
37-
Please return error details for #{oci_error_status} in #{inspect(ctx)}
38-
"""
39-
40-
error_resp(conn, oci_error_status, you_suck_and_are_a_bad_person_messsage)
41-
4233
{:error, oci_error_status, details} ->
4334
error_resp(conn, oci_error_status, details)
4435
end
@@ -55,7 +46,7 @@ defmodule OCI.Plug.Handler do
5546
repo :: String.t(),
5647
id :: String.t(),
5748
ctx :: OCI.Context.t()
58-
) :: Plug.Conn.t() | {:error, atom()} | {:error, atom(), String.t()}
49+
) :: Plug.Conn.t() | {:error, atom(), map() | String.t()}
5950
defp dispatch(%{method: "GET"} = conn, :tags_list, registry, repo, _id, ctx) do
6051
pag = pagination(conn.query_params)
6152

@@ -179,7 +170,7 @@ defmodule OCI.Plug.Handler do
179170
error_resp(
180171
conn,
181172
:BLOB_UPLOAD_INVALID,
182-
"Content-Range header is required for PATCH requests"
173+
%{reason: "Content-Range header is required for PATCH requests"}
183174
)
184175

185176
_ ->
@@ -325,7 +316,7 @@ defmodule OCI.Plug.Handler do
325316
method = conn.method
326317
path = conn.request_path
327318

328-
{:error, :UNSUPPORTED, "Unsupported [#{method}] #{path}"}
319+
{:error, :UNSUPPORTED, %{method: method, path: path}}
329320
end
330321

331322
defp pagination(params) do
@@ -366,7 +357,6 @@ defmodule OCI.Plug.Handler do
366357
ctx
367358
) do
368359
{:ok, _, _} -> :ok
369-
{:error, reason} -> {:error, reason}
370360
{:error, reason, details} -> {:error, reason, details}
371361
end
372362
end

lib/oci/registry.ex

Lines changed: 37 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -80,14 +80,13 @@ defmodule OCI.Registry do
8080
end
8181

8282
@spec validate_repository_name(registry :: t(), repo :: repo_t()) ::
83-
{:ok, repo :: repo_t()} | {:error, :NAME_INVALID, String.t()}
83+
{:ok, repo :: repo_t()} | {:error, :NAME_INVALID, map()}
8484

8585
def validate_repository_name(registry, repo) do
8686
if Regex.match?(registry.repo_name_pattern, repo) do
8787
{:ok, repo}
8888
else
89-
{:error, :NAME_INVALID,
90-
"Invalid repo name: #{repo}, must match pattern: #{inspect(registry.repo_name_pattern)}."}
89+
{:error, :NAME_INVALID, %{repo: repo, pattern: inspect(registry.repo_name_pattern)}}
9190
end
9291
end
9392

@@ -149,12 +148,11 @@ defmodule OCI.Registry do
149148
- `{:error, reason}` if the upload fails
150149
"""
151150
def upload_blob_chunk(%{storage: storage}, repo, uuid, chunk, maybe_chunk_range, ctx) do
152-
reg = adapter(storage)
153-
154-
with true <- reg.upload_exists?(storage, repo, uuid, ctx),
155-
{:ok, size} <- reg.get_blob_upload_offset(storage, repo, uuid, ctx),
151+
with :ok <- ensure_upload_exists(storage, repo, uuid, ctx),
152+
{:ok, size} <- adapter(storage).get_blob_upload_offset(storage, repo, uuid, ctx),
156153
:ok <- verify_upload_order(size, maybe_chunk_range),
157-
{:ok, range} <- reg.upload_blob_chunk(storage, repo, uuid, chunk, maybe_chunk_range, ctx) do
154+
{:ok, range} <-
155+
adapter(storage).upload_blob_chunk(storage, repo, uuid, chunk, maybe_chunk_range, ctx) do
158156
{:ok, blobs_uploads_path(repo, uuid), range}
159157
end
160158
end
@@ -185,22 +183,26 @@ defmodule OCI.Registry do
185183
do: {:error, :DIGEST_INVALID, %{repo: repo, uuid: uuid}}
186184

187185
def complete_blob_upload(%{storage: storage}, repo, uuid, digest, ctx) do
188-
with true <- adapter(storage).upload_exists?(storage, repo, uuid, ctx),
186+
with :ok <- ensure_upload_exists(storage, repo, uuid, ctx),
189187
:ok <- adapter(storage).complete_blob_upload(storage, repo, uuid, digest, ctx) do
190188
{:ok, blobs_digest_path(repo, digest)}
191189
end
192190
end
193191

194192
def cancel_blob_upload(%{storage: storage}, repo, uuid, ctx) do
195-
adapter(storage).cancel_blob_upload(storage, repo, uuid, ctx)
193+
with :ok <- ensure_upload_exists(storage, repo, uuid, ctx) do
194+
adapter(storage).cancel_blob_upload(storage, repo, uuid, ctx)
195+
end
196196
end
197197

198198
def blob_exists?(%{storage: storage}, repo, digest, ctx) do
199199
adapter(storage).blob_exists?(storage, repo, digest, ctx)
200200
end
201201

202202
def get_blob(%{storage: storage}, repo, digest, ctx) do
203-
adapter(storage).get_blob(storage, repo, digest, ctx)
203+
with :ok <- ensure_repo_exists(storage, repo, ctx) do
204+
adapter(storage).get_blob(storage, repo, digest, ctx)
205+
end
204206
end
205207

206208
def delete_blob(%{enable_blob_deletion: false}, repo, digest, _ctx),
@@ -298,18 +300,18 @@ defmodule OCI.Registry do
298300
def referenced_blobs(_manifest), do: []
299301

300302
def get_manifest(%{storage: storage}, repo, reference, ctx) do
301-
adapter(storage).get_manifest(storage, repo, reference, ctx)
303+
with :ok <- ensure_repo_exists(storage, repo, ctx) do
304+
adapter(storage).get_manifest(storage, repo, reference, ctx)
305+
end
302306
end
303307

304308
def manifest_exists?(%{storage: storage}, repo, reference, ctx) do
305309
adapter(storage).manifest_exists?(storage, repo, reference, ctx)
306310
end
307311

308312
def list_tags(%{storage: storage}, repo, pagination, ctx) do
309-
if repo_exists?(storage, repo, ctx) do
313+
with :ok <- ensure_repo_exists(storage, repo, ctx) do
310314
adapter(storage).list_tags(storage, repo, pagination, ctx)
311-
else
312-
{:error, :NAME_UNKNOWN, %{repo: repo}}
313315
end
314316
end
315317

@@ -366,17 +368,14 @@ defmodule OCI.Registry do
366368
Returns {:ok, location} on success, {:error, :BLOB_UNKNOWN} if the source blob doesn't exist.
367369
"""
368370
def mount_blob(%__MODULE__{storage: storage} = registry, repo, digest, from_repo, ctx) do
369-
if repo_exists?(storage, from_repo, ctx) do
371+
with :ok <- ensure_repo_exists(storage, from_repo, ctx) do
370372
if blob_exists?(registry, from_repo, digest, ctx) do
371-
# credo:disable-for-next-line
372373
with :ok <- adapter(storage).mount_blob(storage, repo, digest, from_repo, ctx) do
373374
{:ok, blobs_digest_path(repo, digest)}
374375
end
375376
else
376377
initiate_blob_upload(registry, repo, ctx)
377378
end
378-
else
379-
{:error, :NAME_UNKNOWN, %{repo: repo}}
380379
end
381380
end
382381

@@ -431,6 +430,25 @@ defmodule OCI.Registry do
431430
"/#{api_version()}/#{repo}/blobs/uploads/#{uuid}"
432431
end
433432

433+
# Precondition checks — used by registry functions to validate state before
434+
# delegating to storage adapters.
435+
436+
defp ensure_repo_exists(storage, repo, ctx) do
437+
if adapter(storage).repo_exists?(storage, repo, ctx) do
438+
:ok
439+
else
440+
{:error, :NAME_UNKNOWN, %{repo: repo}}
441+
end
442+
end
443+
444+
defp ensure_upload_exists(storage, repo, uuid, ctx) do
445+
if adapter(storage).upload_exists?(storage, repo, uuid, ctx) do
446+
:ok
447+
else
448+
{:error, :BLOB_UPLOAD_UNKNOWN, %{repo: repo, uuid: uuid}}
449+
end
450+
end
451+
434452
@doc """
435453
Calculates the range of a chunk of data.
436454
## Examples

0 commit comments

Comments
 (0)