Skip to content

feat: claim a guest's data on the next real sign-in - #168

Merged
anilcancakir merged 2 commits into
mainfrom
feature/guest-claim
Sep 26, 2026
Merged

anilcancakir merged 2 commits into
mainfrom
feature/guest-claim

Conversation

@anilcancakir

Copy link
Copy Markdown
Member

What is added

  • MagicStarterGuestClaim and GuestClaimOutcome: record a guest's token on its sign-in, then claim POST /auth/guest/claim on the next real sign-in or restore, over a bare driver the devtools do not record. One claim per session is shared by concurrent callers and bound to the session that started it. A 2xx response marks the claim claimed; a 404 or 422 marks it refused and forgotten; anything else keeps the record for a later attempt. The record is forgotten on every sign-out before Auth.logout() returns.
  • The onGuestClaimed hook (a bootstrap argument, useGuestClaimed).
  • Docs, a README row and a CHANGELOG entry.

Uses only magic API already on magic master (sibling-compile).

Sibling-compile note

No version bump and no publish. This targets main for local override consumption by watchools while its adoption PR is open.

Naming collision note

GuestClaimOutcome collides by name with watchools master's own copy of the same type. A local override pointing watchools at a starter checkout carrying this PR breaks watchools master until the watchools adoption PR merges. Hosted builds are protected by the caret, so this only affects override-based local builds.

Local gates

1626 tests, analyze and format clean.

Add MagicStarterGuestClaim and GuestClaimOutcome: record a guest's
token on its sign-in, then claim POST /auth/guest/claim on the next
real sign-in or restore, over a bare driver the devtools do not
record. One claim per session is shared by concurrent callers and
bound to the session that started it. A 2xx response marks the claim
claimed, a 404 or 422 marks it refused and forgotten, anything else
keeps the record for a later attempt. The record is forgotten on every
sign-out before Auth.logout() returns.

Add the onGuestClaimed hook (a bootstrap argument, useGuestClaimed),
docs, a README row and a CHANGELOG entry.
@codecov

codecov Bot commented Sep 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@kodizm

kodizm Bot commented Sep 26, 2026

Copy link
Copy Markdown

Note

Kodizm (AI-generated). May contain mistakes; verify before acting.

The guest-claim flow looks sound and well tested. I found one gap: the error-handling promise in the docs doesn't match the code for callers who await claimIfPending() themselves.

Major

lib/src/http/magic_starter_guest_claim.dart:257 (correctness): The docs say claimIfPending() never throws on ordinary paths and that a transport failure returns GuestClaimOutcome.none. But _claimOnce only catches MagicVaultException. If the bare DioNetworkDriver.post throws (a timeout or an offline socket), the error reaches the host code that does await MagicStarterGuestClaim.instance.claimIfPending(), which is the usage doc/basics/authentication.md recommends. I couldn't read magic's driver source here, so I can't confirm it throws rather than returning a response. MagicStarterAuthController already catches TimeoutException and SocketException around its own requests, though, so that seems likely. No test covers a driver that throws. The background path (claimInBackground) is safe, because catchError logs the error.

Tests

test/http/magic_starter_guest_claim_test.dart runs the flows through real Auth.login, Auth.logout and restore calls. It covers recording, promotion, 2xx/404/422/500 answers, sharing one claim between concurrent callers, the session guard after sign-out, vault refusals, reporting to the hook once, the feature being off, and bootstrap and reset. It does not cover a transport exception from the driver.

CI

  • Lint & Test: success
  • Published graph: success
  • codecov/patch: success
  • Auto-merge low-risk Dependabot PRs: skipped

_claimOnce only caught MagicVaultException, so an injected driver that
throws would propagate past claimIfPending() to a host awaiting it, even
though the doc promises it never throws on ordinary paths. Wrap the
driver's post call in its own try/catch, log any exception with
Log.error, and answer GuestClaimOutcome.none without touching the record
so the next sign-in or restore retries.

Updates the claimIfPending doc to state precisely that the configured
driver turns transport failures into responses and that any exception
the driver still raises is logged and answered as none.
@anilcancakir

Copy link
Copy Markdown
Member Author

The default DioNetworkDriver already converts every DioException into a MagicResponse in each verb (magic/lib/src/network/drivers/dio_network_driver.dart:200, via _handleError), so a timeout or a socket error on the real, configured driver already arrives as a non-2xx response and the record is kept. The gap was narrower: the driver is injectable (MagicStarterGuestClaim({NetworkDriver Function()? driver})), and an injected driver that throws instead of resolving to a response would propagate past claimIfPending() to a host awaiting it, contradicting the doc's promise that it never throws on ordinary paths.

Fixed in 8ba8697: _claimOnce now wraps the driver's post call in its own try/catch, logs any exception with Log.error, and answers GuestClaimOutcome.none without touching the record, so the next sign-in or restore retries. The MagicVaultException handling around the rest of the method is unchanged. The doc on claimIfPending now states precisely that the configured driver turns transport failures into responses and that any exception the driver still raises is logged and answered as none.

New test: a driver that throws answers none, keeps the record, and does not throw at a host awaiting claimIfPending directly in test/http/magic_starter_guest_claim_test.dart. It injects a FakeNetworkDriver whose stub throws a StateError, confirms claimIfPending() completes with none while both Vault keys survive and one error is logged, then confirms a later claim with a working driver still posts the record.

@kodizm review

@kodizm

kodizm Bot commented Sep 26, 2026

Copy link
Copy Markdown

Note

Kodizm (AI-generated). May contain mistakes; verify before acting.

8ba8697 fixes the one finding from my last review. I have no new findings, and the change looks ready to merge.

Changes since the last review: 8ba8697 touches only lib/src/http/magic_starter_guest_claim.dart and its test. I re-read that commit rather than the whole pull request.

  • The earlier finding is resolved. You pointed out that the default DioNetworkDriver already turns transport errors into responses, so the gap was only an injected driver that throws. _claimOnce now puts _driver().post(...) in its own try/catch. The catch covers both building the driver and the post itself. It logs the error with Log.error, using the same message-plus-stack-trace style as MagicStarterAuthController, and returns GuestClaimOutcome.none. It does not call _spend, so the record stays for the next attempt. The existing MagicVaultException handling is unchanged, and the updated doc on claimIfPending now describes what the code does.

Tests

The new test in test/http/magic_starter_guest_claim_test.dart uses a driver that throws a StateError. It checks four things: the host awaiting the call gets none, both Vault keys survive, exactly one error is logged, and a later claim with a working driver still posts once and reports claimed.

CI

  • Lint & Test: success
  • Published graph: success
  • codecov/patch: success
  • Auto-merge low-risk Dependabot PRs: skipped

@anilcancakir
anilcancakir merged commit 539079b into main Sep 26, 2026
5 checks passed
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.

1 participant