Skip to content

feat(platform): give AuthState::LoginFailed a typed kind - #401

Open
filvecchiato wants to merge 1 commit into
mainfrom
feat/typed-login-failure
Open

feat(platform): give AuthState::LoginFailed a typed kind#401
filvecchiato wants to merge 1 commit into
mainfrom
feat/typed-login-failure

Conversation

@filvecchiato

Copy link
Copy Markdown
Collaborator

Closes #390.

  • LoginFailed { kind, reason } with LoginFailureKind::{NoFreeAllowanceSlots, Other}. Hosts branch on kind and use reason as display copy.
  • The wallet reports refusals as prose over EncryptedResponse::Failed(String), so the core recovers the discriminant once in runtime/login_failure.rs instead of leaving every host to regex it.
  • That text comes from this workspace's own SlotError Display impls, 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 kind means hosts won't need a second API change when it lands.

@filvecchiato
filvecchiato requested a review from a team August 14, 2026 13:41

@Imod7 Imod7 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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"];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AuthState::LoginFailed carries only free text, so hosts must regex-match prose to classify a failure

3 participants