Skip to content

refactor(kit): APIError discards structured error bodies, so 400/409 details are unreachable #103

Description

@Adron

Surfaced twice in one day, from opposite ends of the codebase. Filing it once.

The gap

APIError keeps only the decoded {error} string from a failed response. Everything else in the body is discarded at APIClient.swift:234, before any caller sees it:

case 400: return .badRequest(serverMessage: serverMessage)

So an error body that carries structured information — the thing the client needs in order to do anything but show a sentence — cannot reach the code that would act on it.

Two concrete callers, both already written around it

1. The list-schema destructive-change guard (PR #87, closing #85). PUT /api/lists/{id}/schema refuses to drop a column that still holds row data, answering 400 with a propertiesWithData array naming the columns. That is a question, not a malfunction — the UI should name the columns and offer the confirmation. ListSchemaConflictDTO models the full shape and nothing can populate it, so the user gets the server's sentence and no column list.

2. The app-settings compare-and-set conflict (PR #102, closing #56). The app-settings family is compare-and-set: a stale baseVersion answers 409 with the current document attached, so the client can show what changed, or merge, or re-base and retry. Today it can only report that a conflict happened.

Neither is a hypothetical. Both are shipped code with a comment explaining why the better behaviour is absent.

Why it has not been fixed in passing

APIError.badRequest(serverMessage: String?) and its siblings carry an associated value that every case .badRequest(let message) in the codebase pattern-matches. Adding a second associated value breaks every one of those sites, so it is not a change to slip into a feature PR — which is exactly why both PRs flagged it and moved on.

Suggested shape

Keep the body alongside the message rather than replacing it:

case badRequest(serverMessage: String?, body: Data?)

…with the existing pattern matches updated, or — less invasive — a parallel accessor that does not change the cases:

extension APIError {
    /// The raw response body, when the failure carried one.
    var responseBody: Data? {  }

    /// Decodes the body as `T`, or `nil` when it was absent or did not match.
    func details<T: Decodable>(as type: T.Type) -> T?
}

The second keeps every call site compiling and gives both callers what they need. Worth weighing which reads better before writing it — the first is more honest about where the data lives, the second is far cheaper.

Acceptance

  • ListsService.updateSchema surfaces the propertiesWithData column names, and the schema editor's confirmation names them.
  • AppSettingsService surfaces the current document from a 409.
  • A decode failure on an error body degrades to today's behaviour — a message with no details — rather than masking the original error.
  • BDD quartet on the accessor: a body that decodes, a body that does not, an error with no body at all, and a non-HTTP failure.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions