feat(platform): give AuthState::LoginFailed a typed kind - #401
feat(platform): give AuthState::LoginFailed a typed kind#401filvecchiato wants to merge 1 commit into
Conversation
Imod7
left a comment
There was a problem hiding this comment.
rust/crates/truapi-host-cli/src/main.rs line 1170 already does this classification:
err.to_string().contains("no free StatementStore slot")With the new classify_login_failure, the same fact lives in two crates under two different rules, and the CLI copy is the weaker one: case-sensitive, and no long-term-storage marker. It is load-bearing, prepare_pairing_response uses it to rotate an exhausted auto-managed account. Could you move the markers plus one predicate into rust/crates/truapi-server/src/runtime/statement_allowance/slot.rs, next to the SlotError Display strings they mirror, and call it from both places. I tried it: pub fn reports_exhausted_period(text: &str) -> bool there, classify_login_failure as a wrapper, and the CLI calling truapi_server::statement_allowance::slot::reports_exhausted_period(&err.to_string()). It compiles and the new tests stay green.
| /// Markers that identify an exhausted statement-store allowance period. Every | ||
| /// `SlotError` variant that means "no slot is available to take" renders one of | ||
| /// these. | ||
| const NO_FREE_SLOT_MARKERS: &[&str] = &["no free statementstore slot", "no free long-term-storage"]; |
There was a problem hiding this comment.
These markers are the lowercase of this workspace's SlotError Display text, but nothing here produces a LoginFailed reason from SlotError. The only consumer of a refusal is v2::EncryptedResponse::Failed(reason) in sso_pairing.rs, and there is no producer: respond_to_pairing only submits Success, and the CLI wallet aborts with ? when register_pairing_allowances fails, so no reason is sent. The text comes from an external wallet, whose wording this repo does not control, and the existing fixture shows the real shape: "The operation couldn't be completed. (SubstrateSdk.JSONRPCError error 1.)". The markers are also narrower than the host regex they replace, /no\s+free.*slot|slot.*available|limit\s*=/i: I ran the classifier and "no free statement store slot in period 20486 (max 8)", "No free slots available (limit=8)" and "no free slot in period 20486" all classify as Other, so a host that drops its regex for kind loses fast-fail. Could you widen the rule to reason.contains("no free") && reason.contains("slot") on the lowercased reason. I checked every SlotError variant: the ones that must stay Other still do.
| inner.cancel_tx = None; | ||
| inner.state = AuthState::LoginFailed { reason }; | ||
| inner.state = AuthState::LoginFailed { | ||
| kind: classify_login_failure(&reason), |
There was a problem hiding this comment.
The classifier has three tests, but nothing tests that it is called here, or that login_failed_before_pairing deliberately does not call it. The two pairing tests were widened to AuthState::LoginFailed { reason, .. }, so they ignore kind, and the stub's only wallet failure string classifies as Other, so the NoFreeAllowanceSlots branch is never exercised past the pure function. Could you add two tests to the mod tests right below in this file, which already has stub_platform: one driving pairing_started, authentication_started, login_failed("no free StatementStore slot in period 7 (max 8)") and asserting the emitted state carries LoginFailureKind::NoFreeAllowanceSlots, and one calling login_failed_before_pairing with the same reason and asserting Other.
| /// than only display it. | ||
| #[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Encode, Decode)] | ||
| #[cfg_attr(feature = "uniffi", derive(uniffi::Enum))] | ||
| pub enum LoginFailureKind { |
There was a problem hiding this comment.
Now that one kind is deterministic for the rest of the period, the host-facing docs that call LoginFailed retryable are wrong. Could you add a clause to each saying it is retryable unless kind is NoFreeAllowanceSlots: rust/crates/truapi-server/src/native.rs line 423 (LoginFailed as a retryable error), ios/truapi-host/Sources/TrUAPIHost/TrUAPIHost.swift line 377, ios/truapi-host/README.md line 150, android/truapi-host/src/main/kotlin/io/parity/truapi/TrUAPIHost.kt line 263, and android/truapi-host/README.md line 165. The two copies in the generated truapi_server.swift come from the native.rs doc, so they follow from the bindings sync.
Closes #390.
LoginFailed { kind, reason }withLoginFailureKind::{NoFreeAllowanceSlots, Other}. Hosts branch onkindand usereasonas display copy.EncryptedResponse::Failed(String), so the core recovers the discriminant once inruntime/login_failure.rsinstead of leaving every host to regex it.SlotErrorDisplayimpls, and the tests classify straight from them — rewording one now fails CI here rather than silently turning a host's fast-fail into a retry loop.Typing the inter-host wire itself is a follow-up: it needs a coordinated wallet rollout, and
kindmeans hosts won't need a second API change when it lands.