-
Notifications
You must be signed in to change notification settings - Fork 25
fix(ui): read SwapKit's error codes on the DEX amount screens #1087
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,20 +20,68 @@ | |
| import Foundation | ||
|
|
||
| /// Maps raw SwapKit error code strings to user-facing messages. | ||
| /// Mirrors Android's `SwapKitErrors.messageResFor`. | ||
| /// Mirrors Android's `SwapKitErrors`. | ||
| /// | ||
| /// SwapKit reports a failure in one of two places, and the code is the only stable identifier in | ||
| /// either: the top-level `error` field (`noRoutesFound`, `validation_error`, …) on a non-2xx body, | ||
| /// or `providerErrors[].errorCode` on a 200 that carries no routes. Provider-level failures reach | ||
| /// this type through `providerErrorMessage(_:)`, which puts the code back in front of the prose so | ||
| /// a single `"<code>: <detail>"` shape covers both — matching on the prose instead would drop | ||
| /// `sellAssetAmountTooSmall` into the generic copy, which is exactly the case where the user needs | ||
| /// to be told to raise the amount. | ||
| enum SwapKitErrorCopy { | ||
| static let mayaMemoTooLongErrorCode = "mayaMemoTooLong" | ||
| /// Top-level code for "no provider can carry this pair/amount", lowercased for matching. | ||
| static let noRoutesFoundCode = "noroutesfound" | ||
|
|
||
| /// A provider-level failure rendered in the same `"<code>: <detail>"` shape the top-level | ||
| /// `error` field uses, so both reach `message(for:coin:minimum:)` through one path. | ||
| /// Nil when the provider reported neither a code nor prose. | ||
| static func providerErrorMessage(_ error: SwapKitProviderError?) -> String? { | ||
| let code = error?.errorCode?.trimmingCharacters(in: .whitespacesAndNewlines) | ||
| let detail = error?.message?.trimmingCharacters(in: .whitespacesAndNewlines) | ||
| switch (code?.isEmpty == false ? code : nil, detail?.isEmpty == false ? detail : nil) { | ||
| case let (code?, detail?): | ||
| return "\(code): \(detail)" | ||
| case let (code?, nil): | ||
| return code | ||
| case let (nil, detail?): | ||
| return detail | ||
| case (nil, nil): | ||
| return nil | ||
| } | ||
| } | ||
|
|
||
| /// True when the failure means "the sell amount is under what this route can fill" — the case | ||
| /// the amount screens surface inline (raise the amount and retry) instead of as a dead end. | ||
| /// `noRoutesFound` is included because SwapKit answers with it for amounts far below the | ||
| /// minimum and only switches to an explicit below-minimum code close to the floor: measured | ||
| /// 2026-08-28, DASH → BTC returned `noRoutesFound` at 0.01 DASH and `sellAssetAmountTooSmall` | ||
| /// (min 0.175) at 0.05. It is genuinely ambiguous — a route can also be briefly unavailable — | ||
| /// so its copy stays neutral. | ||
| static func isAmountTooLow(_ rawError: String?) -> Bool { | ||
| let code = code(of: rawError) | ||
| return code == noRoutesFoundCode || isBelowMinimumCode(code) | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💬 Nitpick: Remove the unused Repository-wide usage shows no caller for this newly added helper. The amount screens call source: ['codex']
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed in 46ebf78. It was added for parity with Android’s |
||
|
|
||
| static func message(for rawError: String?, coin: SwapCryptoCurrency) -> String { | ||
| let code = rawError? | ||
| .components(separatedBy: ":") | ||
| .first? | ||
| .trimmingCharacters(in: .whitespacesAndNewlines) | ||
| .lowercased() | ||
| ?? "" | ||
| let code = code(of: rawError) | ||
|
|
||
| // Per-provider below-minimum codes are family-matched rather than enumerated: SwapKit does | ||
| // not document the per-provider vocabulary, and the prefix names whichever side was too | ||
| // small (`sellAssetAmountTooSmall` today). Both forms carry "amount", so an unrelated | ||
| // below-threshold code — a too-low fee, say — stays out. | ||
| if isBelowMinimumCode(code) { | ||
| return NSLocalizedString( | ||
| "This amount is below the minimum for this swap. Please enter a larger amount.", | ||
| comment: "Dash DEX / dex_error_amount_too_small" | ||
| ) | ||
| } | ||
|
Comment on lines
+40
to
+67
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Suggestion: Add regression tests for the new SwapKit error normalization This bug fix introduces several interacting parsing and classification rules without executable coverage. Extend the existing SwapKit tests with compile-ready cases for provider code/detail composition, code-only and detail-only responses, whitespace and nil handling, code-first top-level decoding, case-insensitive source: ['claude', 'codex']
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added in 46ebf78 — a The anchor case is the regression itself: Two limits worth stating plainly:
|
||
|
|
||
| switch code { | ||
| case "mayamemotoolong": | ||
| // `memoTooLongForSourceChain` is SwapKit's own name for what the wallet also detects | ||
| // locally before broadcasting; both mean the OP_RETURN will not fit. | ||
| case "mayamemotoolong", "memotoolongforsourcechain": | ||
| // Two things drive the memo past the 80-byte OP_RETURN limit: the destination | ||
| // address, and the amount-dependent streaming-limit field. Measured on 2026-08-04, | ||
| // one ARB.YUM route to a fixed address ran 79 / 80 / 79 / 79 bytes at 0.1 / 1 / 10 / | ||
|
|
@@ -43,12 +91,12 @@ enum SwapKitErrorCopy { | |
| "This swap's Maya instruction doesn't fit in a Dash transaction. Try a different amount, or a shorter %@ address.", | ||
| comment: "Dash DEX / dex_error_maya_memo_too_long" | ||
| ), chainLabel) | ||
| case "noroutesfound": | ||
| case noRoutesFoundCode: | ||
| return NSLocalizedString( | ||
| "This amount can't be swapped right now. Routes can be briefly unavailable — try again shortly, or try a different amount.", | ||
| comment: "Dash DEX / dex_error_no_route" | ||
| ) | ||
| case "blacklistasset": | ||
| case "blacklistasset", "invalidasset": | ||
| return String(format: NSLocalizedString( | ||
| "%@ can't be swapped at the moment.", | ||
| comment: "Dash DEX / dex_error_blacklisted" | ||
|
|
@@ -58,7 +106,9 @@ enum SwapKitErrorCopy { | |
| "We couldn't set up your swap. Please check the amount and address, then try again.", | ||
| comment: "Dash DEX / dex_error_validation" | ||
| ) | ||
| case "apikeyinvalid", "unauthorized": | ||
| // `apiRequestFailed` is SwapKit failing to reach the provider it quotes through — an | ||
| // upstream outage from the user's side, same as an unavailable swap desk. | ||
| case "apikeyinvalid", "unauthorized", "apirequestfailed": | ||
| return NSLocalizedString( | ||
| "Swaps are temporarily unavailable. Please try again later.", | ||
| comment: "Dash DEX / dex_error_unavailable" | ||
|
|
@@ -83,7 +133,9 @@ enum SwapKitErrorCopy { | |
| "This token needs approval before it can be swapped.", | ||
| comment: "Dash DEX / dex_error_allowance" | ||
| ) | ||
| case "unabletobuildtransaction": | ||
| // `invalidRoute` is a quoted route SwapKit then refuses to execute — nothing the user can | ||
| // correct, and retrying re-quotes, so it takes the same copy as a failed build. | ||
| case "unabletobuildtransaction", "invalidroute": | ||
| return NSLocalizedString( | ||
| "We couldn't prepare this swap. Please try again.", | ||
| comment: "Dash DEX / dex_error_build_failed" | ||
|
|
@@ -115,4 +167,34 @@ enum SwapKitErrorCopy { | |
| ) | ||
| } | ||
| } | ||
|
|
||
| /// True when the failure is the top-level "no provider can carry this pair/amount" code — | ||
| /// which SwapKit also reports per provider inside `providerErrors[]`. | ||
| static func isNoRoute(_ rawError: String?) -> Bool { | ||
| code(of: rawError) == noRoutesFoundCode | ||
| } | ||
|
|
||
| /// True when the failure is specifically "under this route's minimum" — the unambiguous half | ||
| /// of [isAmountTooLow]. Routability probing needs this narrower test: a provider-level | ||
| /// `noRoutesFound` really does mean the provider can't carry the asset, whereas a | ||
| /// below-minimum reply only says the probe amount was too small. | ||
| static func isBelowMinimum(_ rawError: String?) -> Bool { | ||
| isBelowMinimumCode(code(of: rawError)) | ||
| } | ||
|
|
||
| private static func isBelowMinimumCode(_ code: String) -> Bool { | ||
| code.hasSuffix("amounttoosmall") || code.hasSuffix("amounttoolow") | ||
| } | ||
|
|
||
| /// The SwapKit code carried by a raw error: the leading token before an optional | ||
| /// `": <detail>"`, lowercased, so a bare `validation_error` and `validation_error: <detail>` | ||
| /// both match. | ||
| private static func code(of rawError: String?) -> String { | ||
| rawError? | ||
| .components(separatedBy: ":") | ||
| .first? | ||
| .trimmingCharacters(in: .whitespacesAndNewlines) | ||
| .lowercased() | ||
| ?? "" | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -346,7 +346,8 @@ final class SwapKitSwapProvider: SwapProvider { | |
| } | ||
|
|
||
| guard let best = bestRoute(from: quoteResponse.routes ?? []) else { | ||
| let msg = quoteResponse.providerErrors?.first?.message | ||
| let providerError = quoteResponse.providerErrors?.first | ||
| let msg = SwapKitErrorCopy.providerErrorMessage(providerError) | ||
| ?? NSLocalizedString("No route available", comment: "SwapKit") | ||
| return errorResult(msg) | ||
| } | ||
|
|
@@ -375,7 +376,8 @@ final class SwapKitSwapProvider: SwapProvider { | |
|
|
||
| // Step 2: pick RECOMMENDED → CHEAPEST → first (mirrors Android bestRoute()). | ||
| guard let best = bestRoute(from: quoteResponse.routes ?? []) else { | ||
| let msg = quoteResponse.providerErrors?.first?.message | ||
| let providerError = quoteResponse.providerErrors?.first | ||
| let msg = SwapKitErrorCopy.providerErrorMessage(providerError) | ||
| ?? NSLocalizedString("No route available", comment: "SwapKit") | ||
| return errorResult(msg) | ||
| } | ||
|
|
@@ -614,7 +616,8 @@ final class SwapKitSwapProvider: SwapProvider { | |
| } | ||
|
|
||
| guard let best = bestRoute(from: quoteResponse.routes ?? []) else { | ||
| let message = quoteResponse.providerErrors?.first?.message | ||
| let providerError = quoteResponse.providerErrors?.first | ||
| let message = SwapKitErrorCopy.providerErrorMessage(providerError) | ||
| ?? quoteResponse.message | ||
| ?? quoteResponse.error | ||
| ?? NSLocalizedString("No route available", comment: "SwapKit") | ||
|
|
@@ -727,16 +730,24 @@ final class SwapKitSwapProvider: SwapProvider { | |
| return .routable | ||
| } | ||
|
|
||
| let message = [response.error, response.message, response.providerErrors?.first?.message] | ||
| .compactMap { $0 } | ||
| .joined(separator: " ") | ||
| // Classify on the codes, not the prose: a provider reports its reason in | ||
| // `providerErrors[].errorCode`, and only the code is a stable identifier. | ||
| let providerCode = SwapKitErrorCopy.providerErrorMessage(response.providerErrors?.first) | ||
|
|
||
| if isConfirmedNoRoute(message) { | ||
| return .notRoutable | ||
| if let providerCode { | ||
| if SwapKitErrorCopy.isBelowMinimum(providerCode) { | ||
| // The probe amount was under this route's floor, which says nothing about whether | ||
| // the asset is routable — the picker must not hide it on this evidence. | ||
| return .routable | ||
| } | ||
|
|
||
| // A provider naming its own no-route is as conclusive as the top-level code; anything | ||
| // else it reports (an upstream `apiRequestFailed`, say) leaves the question open. | ||
| return SwapKitErrorCopy.isNoRoute(providerCode) ? .notRoutable : nil | ||
| } | ||
|
|
||
| if isMinimumOrAmountError(message) { | ||
| return .routable | ||
| if SwapKitErrorCopy.isNoRoute(response.error) { | ||
| return .notRoutable | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Suggestion: Do not classify top-level The PR's recorded API behavior shows that SwapKit returns top-level source: ['codex']
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 46ebf78 — the top-level branch is gone and that response now falls through to Agreed on the reasoning, and the recorded data is worse than the comment suggests: DASH → BTC returns top-level Provider-level evidence still decides: a below-minimum code is positive, a provider naming its own |
||
|
|
||
| return nil | ||
|
|
@@ -747,27 +758,21 @@ final class SwapKitSwapProvider: SwapProvider { | |
| return try? JSONDecoder().decode(SwapKitQuoteResponse.self, from: response.data) | ||
| } | ||
|
|
||
| private static func isConfirmedNoRoute(_ message: String) -> Bool { | ||
| let normalized = message.lowercased() | ||
| return normalized.contains("noroutesfound") || normalized.contains("no routes found") | ||
| } | ||
|
|
||
| private static func isMinimumOrAmountError(_ message: String) -> Bool { | ||
| let normalized = message.lowercased() | ||
| return normalized.contains("below minimum") | ||
| || normalized.contains("amount too small") | ||
| || normalized.contains("too small") | ||
| || normalized.contains("minimum") | ||
| } | ||
|
|
||
| private func decodeQuoteError(from error: Error) -> String? { | ||
| guard case HTTPClientError.statusCode(let response) = error, | ||
| let body = try? JSONDecoder().decode(SwapKitQuoteResponse.self, from: response.data) | ||
| else { | ||
| return nil | ||
| } | ||
|
|
||
| return body.message ?? body.error ?? body.providerErrors?.first?.message | ||
| if let code = body.error, !code.isEmpty { | ||
| if let message = body.message, !message.isEmpty { | ||
| return "\(code): \(message)" | ||
| } | ||
| return code | ||
| } | ||
|
|
||
| return SwapKitErrorCopy.providerErrorMessage(body.providerErrors?.first) ?? body.message | ||
| } | ||
|
|
||
| private func decodeSwapError(from error: Error) -> String? { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💬 Nitpick: Correct the nonexistent mapper reference in the doc comment
The comment references
message(for:coin:minimum:), but this module only definesmessage(for:coin:). Point the documentation at the actual mapper so readers are not directed to a nonexistent overload.source: ['codex']
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 46ebf78. The same slip existed a second time —
isBelowMinimum(_:)referred to[isAmountTooLow], which your other comment asked me to delete — so both now point at symbols that exist.