fix: make QR-code login and endpoint switch reliable

[deaflynx]

Fixes the QR-code login / endpoint switch flow. Related: PROD-8200, thingsboard/flutter_thingsboard_pe_app#303.

Root causes

• Login and NotificationService captured the ThingsboardClient once at construction. After a QR switch re-creates the client (ITbClientService.reInit), they kept calling the old host with the new host's tokens → 401 Token is outdated, and the failed refresh wiped the fresh tokens from shared storage, undoing the switch. A full app restart "fixed" it because the client was re-captured.
• switchEndpoint loaded the new tokens into the old client before switching and rolled back to the compiled default endpoint (not the previous one) on failure, losing the user's session.
• reInit's onAuthError re-threw from inside the error callback → unhandled zone errors, and the noauth view rendered ThingsboardError.toString(), which embeds the stacktrace shown on screen.
• A QR link without a secret (the mobile-app QR on the web login page) rendered a spinner forever.
• The v2 router never subscribes to app links (TbContext.init is no longer called), so camera-scanned links were silently dropped.
• Re-scanning the same QR after a logout: the server returns the same, already-revoked JWT pair (bound to the secret), sending the app into a silent login/refresh loop.
• Best-effort calls right after login (getLoginMobileInfo, notification mobile-session sync, the client's internal version check) surfaced their expected 401/403 answers as "You don't have permission" toasts.

Changes

• Read the live client via getter in Login and NotificationService.
• Rework NoauthProvider.switchEndpoint: stage the exchanged pair in storage before reInit (the new client can only start with exactly these tokens), verify the pair with GET /api/auth/user on the target host before committing (a revoked pair now fails within seconds and rolls back), roll back to the previous endpoint on failure keeping the previous session, classify failures into localized copy and fall back to the server's message only for answers the app cannot classify.
• Handle no-secret QR links as a host-only switch that lands on the new host's login page; SwitchEndpointArgs.secret is now optional.
• Never render stacktraces in the noauth view; navigate on completion (login page for host-only switches, home for already-authenticated duplicates) with a fallback so the user is never stranded on the spinner.
• Restore app-link handling in the app root (stream + cold-start link, platform duplicate deliveries dropped).
• Pop the QR scanner once per scan (repeated MLKit detections crashed with GoError: There is nothing to pop).
• Mark best-effort requests (getLoginMobileInfo, mobile-session calls, unread count) with ignoreErrors/ignoreLoading extras and suppress client-internal init errors so expected 401/403 answers no longer toast; guard fire-and-forget handleUserLoaded against unhandled async errors.

Testing

Verified on emulator (Android 16) against a local TB instance and demo.thingsboard.io, driving the same navigateByAppLink path the in-app scanner uses, plus on-device scanner runs:

• form login, QR switch to another host with auto-login, same-host QR relogin, re-scan of the same QR within TTL (logs in again), re-scan after logout (clean server message + rollback, session-preserving), expired/invalid secret (server message + rollback), no-secret QR cross-host (lands on the new host's login page) and same-host (returns home), logout/login regression
• zero unhandled exceptions across all runs, flutter analyze at the develop/1.9.0 baseline

Review follow-up

d57ab30 (first round)

Behavioural findings from the review, fixed:

• The rollback restores the session, not just the endpoint. The exchanged pair was staged in storage before reInit, so any later failure left reset re-initialising the previous host with the new host's tokens — 401, failed refresh, wiped session, i.e. the very failure this PR is about, reached from the rollback path. Both token keys are now read into locals before the first write and restored on failure; the same covers _switchHostOnly, which clears them via setUserFromJwtToken(null, null, false).
• No stacktrace can reach the screen. ThingsboardError.message is nullable and the old fallback chain ended at toString(), which embeds the captured stacktrace. Failures now travel as a NoAuthFailure enum plus an optional server-supplied message, and the view resolves them — the provider never stores toString().
• The navigation provider is primed before it is read. Navigation.build() does not populate _pagesLayout, so a notifier built for the first time returned an empty page list; being under four items it got the Profile item appended, landing the user on fullscreen Profile instead of home. Navigation.resolveHomePath() primes and returns the first tab, with LoginRoutes.login as the fallback the empty-list branch previously lacked; HomeHandler uses the same call.
• The init-time error suppression is counted, not a flag, so an overlapping init (a QR switch during a cold-start init) can no longer lift another's window. It stays time-based: ThingsboardError carries no request path, so onClientError cannot narrow it to the specific /api/admin/updates answer. init() and reInit() now share one _initClient().
• The post-switch fallback no longer races HomeHandler. The 12 s timer is held and cancelled as soon as loginProvider reports fully authenticated; the 5 s error dismissal is a separate timer so the two cannot cancel each other, and both are cancelled on dispose. The error screen also got a Go back action instead of a forced wait.
• navigateTo percent-encodes its arguments. The query was built by hand, so a value containing & — an app link passed as uri always does — was re-split on its own separator. Benign today, but it is what decides whether uri survives, and the host-less link case depends on it.

Structure and consistency:

• switchEndpoint is split into named steps — _exchangeSecret, _verifySession, _installSession, _rollbackTo — with shared _reInitClient and _asFailure helpers replacing three copies of the reInit call and two of the DioException mapping. isTheSameHost is derived once and the same-host short-circuit hoisted out of _switchHostOnly; reset is now the private _rollbackTo(endpoint, session).
• The two storage keys the client reads are named constants with a comment tying them to the pinned client version. The client exposes no supported way to seed a session and no key constants, so the coupling stays — it is now discoverable in one place.
• The switch copy is localized: a step/failure enum in NoAuthState resolved through S.of(context), with five new keys in intl_en.arb. Other locales fall back to English until translated.
• App-link handling moved into useAppLinks() — one subscription, one place for the policy — and the dead second subscription in TbContext is deleted (init() is unreachable: the init widgets are never instantiated). The 2 s duplicate window is kept with a named constant: tracking only the last link would permanently swallow a deliberate re-scan of the same QR, which is a supported case.
• A shared request-extra helper (since renamed to silentRequestExtra(), see below) is used by oauth_provider and notification_service, and documents that it suppresses the UI only, not the client's refresh-and-clear.
• Smaller: LoginRoutes.login instead of a hardcoded path (4 sites), non-nullable arguments in the view (its null branch and ! were dead), a named constant for the app-link uri query parameter, corrected the 403 rationale on getLoginMobileInfo, and dropped the unread ttl field.

5579b78, a2e175c (second round)

• Localized copy wins over server text. The server's messages are hardcoded English and say "Token has expired" for a pair revoked by logout. tokenExchangeFailed and sessionInvalid always render the localized strings; serverMessage is shown only for NoAuthFailure.unknown.
• Malformed links end on the error screen. Host resolution moved inside the try (_resolveHost, guarded on http(s) schemes instead of isAbsolute), so a non-http scheme or an empty host rolls back and shows the error instead of hanging the view on the spinner.
• Connection errors stay visible inside the init-time suppression window. An unreachable server is never what the init-time 401/403 answers look like.
• App links: stream errors are logged, and the duplicate window is fixed, not sliding.
• One spelling of the Dio flag. ThingsboardAppConstants.ignoreErrors is deleted and the two 2FA call sites use the shared helper. They now also opt out of the global loading indicator; the page drives its own.
• The scanner stops the camera once a code is handled, instead of decoding frames until dispose.
• The same-server check compares origins, not hosts (a2e175c). A link that differs from the current endpoint only by scheme or port now performs the switch; before, it did nothing and left the user on the old host.
• Smaller: _installSession takes _ExchangedSession, keeping the non-null token guarantee; NoAuthState.error is gone (failure is the signal); the dead HomeHandler.init block and its unused imports are deleted; the storage-key comment names the path dependency instead of a pin that does not exist.

c05f5e7, 6db5d02, 272d62b, 3cddf46 (third to sixth rounds)

• Failures are classified by response, once, in _asFailure. No response at all is connectionFailed, rendered as "Failed to connect to {host}" (failedToConnectToHost, the sixth key added to intl_en.arb, other locales fall back to English). A 401 is the caller's rejection kind: tokenExchangeFailed on /api/noauth/qr/{secret} (the path is permitAll, so a 401 there can only be a rejected secret) and sessionInvalid on /api/auth/user (the server maps JWT_TOKEN_EXPIRED to 401). Any other status is unknown and the server's message is shown. A 20 s timeout against the target host no longer asks the user to scan a new QR code. SwitchEndpointFailure.status and cause (the DioExceptionType) are diagnostics only: they reach the log through toString(), so a timeout, a refused connection and a bad certificate are distinguishable there. Call sites pass the rejection kind as rejectedAs:.
• _isSameOrigin is asymmetric by design. An unusable target throws and ends on the error screen; an empty or schemeless stored endpoint (no dart-define, or carried over from an older build) counts as a different server so the switch proceeds. EndpointService.isCustomEndpoint keeps comparing hosts on purpose: Firebase is bound to the default host, not to a scheme or port.
• bestEffortRequestExtra() is renamed to silentRequestExtra() (lib/utils/silent_request.dart): it opts a call out of both the global error overlay and the global loading indicator, and the 2FA callers are not best-effort. TbClientService._shouldSuppress(e) owns the init-time suppression rule.
• Housekeeping (3cddf46): dio is declared directly in pubspec.yaml (it was resolved transitively through the client and is now a field type on SwitchEndpointFailure), and login_provider.g.dart is regenerated: the PR changed login_provider.dart without refreshing its riverpod hash. flutter analyze is at 130 issues against 137 on develop/1.9.0, zero new; every generated file reproduces from build_runner and intl_utils:generate.

Still open:

• ignoreErrors does not stop the refresh-and-clear. Confirmed in the client: a 401 with jwtTokenExpired takes _refreshTokenAndRetry regardless of the interceptor config, and its failure path calls _handleError(..., true) with notify hardcoded — so a best-effort call can still both toast and clear the staged session. The fix belongs in http_interceptor.dart (notify && !config.ignoreErrors, plus an opt-out flag if the refresh itself should be skipped) and needs its own client PR.
• No tests. Every collaborator comes from getIt and the Dio instance is built inline, so the branches are not reachable from a test yet; the switch flow needs IEndpointService/ITbClientService/an HTTP data source injected through the notifier first.
• Storage key names are duplicated from the client. jwt_token/refresh_token are named constants here because the client exports none. Exporting them from thingsboard_client_base.dart would turn the "must be kept in sync" comment into a compile error; cheap to fold into the client PR above.

[vvlladd28]

Review summary

Reviewed 13 changed files in fix: make QR-code login and endpoint switch reliable (PROD-8200). Left 22 comment(s) inline.

The staging-before-reInit approach is the right call and I verified it against the client library (init() does read jwt_token/refresh_token from storage, so the new client really does start with exactly the exchanged pair). Three things stood out as behavioural gaps rather than style: the rollback path no longer preserves the previous session once the tokens have been staged (contradicting reset's own doc comment), the stacktrace guard in the noauth view is defeated by its own fallback when ThingsboardError.message is null, and ignoreErrors only suppresses the toast — it does not stop a 401 from triggering the refresh-and-wipe that this PR is fixing.

Verification caveat. The branch depends on thingsboard_ce_client via a path dependency (../thingsboard-dart-client/ce), which is not present on this machine. Claims about client internals — the interceptor's refresh-then-wipe behaviour, ThingsboardError.toString() embedding the stacktrace, and init() reading jwt_token/refresh_token from storage — were verified against thingsboard_client 4.1.0 and the dart_thingsboard_client source, which share the same handwritten core API surface the app calls (init, setUserFromJwtToken, isAuthenticated, InterceptorConfig.toExtra) but not the generated controller APIs. Treat those as high-confidence, not proven, on the exact dependency. Everything in the app layer, and every platform-side claim, was verified directly.

Additional findings

These observations are about existing code outside the PR's diff — spotted while reading surrounding context.

• lib/config/routes/router.dart:69 — navigateTo builds the query string by hand: mapArgs.entries.map((e) => '${e.key}=${e.value}').join('&'), with no percent-encoding, and the new 'uri' fallback in noauth_routes.dart now leans on it. With a real deep link (…/api/noauth/qr?secret=X&ttl=120&host=https://demo.thingsboard.io, per DEEP_LINK_PATTERN in QrCodeSettingsController.java:90) the uri value is re-split on its own &, so queryParameters['uri'] comes back truncated to …/api/noauth/qr?secret=X and ttl/host reappear as duplicate top-level keys. This is benign today — the truncated value still yields the right uri.origin, and the duplicates carry identical values — so it's latent fragility rather than a live bug. Worth noting the secret itself can't break the parse: generateSafeToken uses Base64.getUrlEncoder().withoutPadding() (StringUtils.java:241), so it's URL-safe with no =. Uri(queryParameters: mapArgs).query would remove the sharp edge regardless.
• lib/core/auth/noauth/presentation/view/switch_endpoint_noauth_view.dart:17 — with the route builder now always constructing args, arguments can never be null at the only construction site, so the nullable type, the else { … context.go('/login') } branch in the effect and the arguments! bang are all dead. Making the field non-nullable would delete all three.
• lib/core/auth/noauth/provider/noauth_provider.dart:60-72 — the inline tempDio block re-implements a slice of the client library by hand: the /api/noauth/qr/… path, the X-Authorization: Bearer header convention, and two bare 20-second timeouts as literals. If the auth header name or that endpoint moves, this is the one place that won't be updated alongside the generated API. Putting it behind a small injectable data source would also give the switch flow the test seam it currently lacks.
• lib/core/context/tb_context.dart:287 — there's a second appLinks.uriLinkStream subscription here (guarded by _appLinkStreamSubscription ??=). It's unreachable today — ThingsboardInitApp and ThingsboardInitRegionApp are never instantiated, so TbContext.init() is dead code in the CE app, which is exactly why this PR needs to re-add the subscription. Worth deleting the dead path, though: if it's ever revived both subscriptions go live and every app link is handled twice, and the new 2s de-duplication window can't see the other handler.

---

This review was auto-generated. Findings may contain errors — please verify before applying changes.

[vvlladd28]

This rollback no longer preserves the session, and reset's doc comment below says the opposite — "The previous session tokens are still in storage (the new ones are only persisted after a successful reInit)". The staging block at line 141 writes the exchanged pair to jwt_token/refresh_token before reInit, so by the time anything after that throws, the previous host's tokens are already gone.

Concretely: setEndpoint, _switchFirebaseApps or reInit throws (init() in the client rethrows as a ThingsboardError) → we land here → reset restores previousEndpoint and re-inits → the client reads storage and finds the new host's tokens → 401 against the old host → _refreshTokenAndRetry → refreshJwtToken fails → _clearJwtToken() wipes storage. That's the exact failure mode described in the PR's root-cause section, just reached from the rollback path instead.

The cross-host _switchHostOnly branch has the same shape — it calls setUserFromJwtToken(null, null, false), which deletes both keys, before reInit; a failure there logs the user out of the host they came from.

Would it be worth reading the two keys into locals before staging and restoring them in reset (or passing them in), so the rollback is a true rollback of both the endpoint and the session?

[deaflynx]

Fixed. Both token keys are read into locals at the top of switchEndpoint, before anything is written, and the rollback is now _rollbackTo(previousEndpoint, previousSession) which restores the endpoint and writes those values back. _switchHostOnly's setUserFromJwtToken(null, null, false) is covered by the same restore, so a failure there no longer logs the user out of the host they came from. The doc comment now describes what the code actually does.

[vvlladd28]

The fallback defeats the guard in the one case it's meant to cover. ThingsboardError.message is nullable, and when it's null this falls through to noAuth.message — which the provider set as e is ThingsboardError ? e.message ?? e.toString() : '$e', i.e. the same null message falls through to e.toString(). And ThingsboardError.toString() is precisely what appends the stacktrace when _stackTrace != null, so the stacktrace still reaches the screen.

The errors this PR raises itself always pass a message, but ones bubbling up from the client (toThingsboardError) don't always, and those are the ones carrying a stacktrace.

A fixed fallback would close it — error.message ?? S.of(context).somethingWentWrong (there's a commented-out somethingWentWrongRollback right below that looks intended for this). Worth fixing the provider's message: at line 181 the same way, since that's the other half of the chain.

Separately: once the fallback is safe in one place, this whole ternary collapses to noAuth.message and the widget stops needing the thingsboard_client import — one place deciding how an error is presented.

[deaflynx]

Fixed at the source rather than in the fallback. The provider no longer stores toString() anywhere: failures travel as a NoAuthFailure enum plus an optional server-supplied message, and _failureState takes error.message when the error is a ThingsboardError. The view resolves the enum through S.of(context) with somethingWentWrong as the terminal fallback, so a null message can't reach toString() from either half of the chain. As you predicted, the ternary collapsed and the thingsboard_client import is gone from the widget.

[vvlladd28]

ignoreErrors is narrower than the comment implies. In the interceptor it only gates the final _handleError(tbError, …, notify && !ignoreErrors). It does not gate the branch above it: a 401 whose errorCode is jwtTokenExpired still takes _refreshTokenAndRetry → refreshJwtToken(interceptRefreshToken: true), and if that refresh fails the client calls _clearJwtToken(), deleting jwt_token/refresh_token from shared storage. That failure path then calls _handleError(e, …, true) with notify hardcoded, so the toast appears anyway.

This is reachable exactly in the scenario the PR is about. On the platform side, logout() → logLogoutAction publishes UserSessionInvalidationEvent(sessionId) (AuthController.java:283), and JwtAuthenticationProvider.java:52 then throws JwtExpiredTokenException("Token is outdated") → JWT_TOKEN_EXPIRED / 401. The refresh that follows hits RefreshTokenAuthenticationProvider.java:62 → CredentialsExpiredException("Token is outdated"). So a best-effort call can both toast and wipe the session that was just staged, with ignoreErrors bypassed on both counts.

The 403 rationale in the comment does check out for these particular calls — the mobile-session endpoints are @PreAuthorize("hasAnyAuthority('SYS_ADMIN', 'TENANT_ADMIN', 'CUSTOMER_USER')") (UserController.java:586-604), so a token outside those authorities gets 403. It's the 401 case that stays open.

Is there a way to skip the refresh for these calls too — a request-level opt-out, or catching at the call site rather than relying on the interceptor? Worth checking whether the client can grow an ignoreAuthRefresh flag, since the app can't reach that decision from here.

Minor: the same InterceptorConfig(ignoreErrors: true, ignoreLoading: true).toExtra() is inlined in oauth_provider.dart too — a shared helper would give both sites one definition.

[deaflynx]

Confirmed, and not fixable from the app. Checked against the client this branch builds against: onError enters _refreshTokenAndRetry for a 401 with jwtTokenExpired regardless of the interceptor config, and its catch calls _handleError(e, ..., true) with notify hardcoded — so with ignoreErrors set, both the session wipe and the toast are still reachable, exactly as you describe.

The fix belongs in http_interceptor.dart: notify && !config.ignoreErrors on that error path, plus an ignoreAuthRefresh-style opt-out if the refresh itself should be skipped for best-effort calls. That's a change to a published library shared by CE/PE/PaaS, so it needs its own client PR and version bump and isn't in this branch — I've listed it as open in the PR description, and the new shared helper documents the limitation at the point of use.

Your 403 rationale for the mobile-session endpoints stands, so the marking is still worth having. The duplicated extra is now bestEffortRequestExtra() in lib/utils/best_effort_request.dart, used by this file and oauth_provider.

[vvlladd28]

This reads navigationProvider without priming it, unlike the other caller. HomeHandler (route_handlers/home_handler.dart:38) calls ref.read(navigationProvider.notifier).onLoggedIn() before ref.read(navigationProvider) — and that call is what populates _pagesLayout via _cachePageLayouts. Navigation.build() itself never primes it, so a notifier built for the first time right here starts with _pagesLayout == [].

In that case _getPages([]) leaves _allPages empty, and because _allPages.length < 4 it still appends the Profile item — so bottomBarPages.first.path is /profile?fullscreen=true and the user lands on fullscreen Profile instead of home. If instead the provider was already built and populated (the common case, since the shell watches it), this works correctly.

Reusing HomeHandler's sequence — onLoggedIn() then read — would make this deterministic, and extracting the shared "go to the first home tab" step would stop the two copies from drifting. Also worth noting the isNotEmpty guard silently does nothing when the list is empty, which is the one isDone branch with no fallback; the else branch below has a timer, this one doesn't.

[deaflynx]

Fixed. Navigation.resolveHomePath() primes via onLoggedIn() and returns bottomBarPages.firstOrNull?.path; the view does context.go(homePath ?? LoginRoutes.login), so this branch now has the fallback it was missing. HomeHandler calls the same method, so the two copies of the sequence can't drift.

[vvlladd28]

The restore isn't guarded against overlap. A QR switch calls reInit while an earlier init/reInit's 2-second timer is still pending: the second call sets the flag to true, then the first timer fires and clears it, so the second init spends most of its life unsuppressed. A generation counter or a depth count (increment on entry, decrement in the timer, only clear at zero) would make it order-independent.

Stepping back — this mutes all client error notifications for a wall-clock window after every init, so genuine failures from unrelated in-flight requests get dropped too, and 2s is a guess about callback scheduling inside the client. It works, but it's the kind of global time-based switch that's hard to reason about later. Given the rest of this PR adopts per-request InterceptorConfig(ignoreErrors: true), could the suppression be scoped instead — filtering in onClientError on the specific endpoint/status we know about (/api/admin/updates → 403), rather than blanket-muting a time slice?

Also the _suppressErrorNotifications = true; await _client.init(); try/finally is now duplicated verbatim in init() and reInit() — worth folding into one _initClient() helper whichever way the suppression lands.

[deaflynx]

Fixed the overlap with a count: _pendingInits is incremented on entry and decremented in the delayed callback, and _suppressErrorNotifications reads > 0, so ordering no longer matters.

Kept it time-based rather than scoping by endpoint/status: ThingsboardError carries only message, errorCode, status and error — no request path — so onClientError has nothing to match /api/admin/updates on. The comment now says that explicitly instead of leaving the reader to wonder, and 2 s is _initErrorSuppression. The duplicated try/finally is folded into one _initClient() used by both init() and reInit().

[vvlladd28]

This depends on an unwritten contract with ThingsboardAppRouter.navigateByAppLink, which flattens its arguments map into a key=value&… string and happens to include a uri key. So uri is now effectively a reserved query parameter of this route, and the fallback silently changes meaning depending on which entry point navigated here.

Worth a comment naming navigateByAppLink as the producer, or better a shared constant for the key, so someone editing either side sees the coupling. (See also the note in the review body about that query string not being percent-encoded — it's what decides whether this uri value survives intact.)

[deaflynx]

Named it: appLinkUriQueryParam lives in router.dart next to the producer, and the route builder reads it from there.

The unencoded query string from your review body is fixed as well — navigateTo now builds the query with Uri(queryParameters: ...).query, so a value carrying its own & survives. That also makes the host-less link case behave as intended: uri.origin now resolves from the full link instead of a truncated one.

[vvlladd28]

While you're reshaping this DTO — ttl isn't read anywhere in the codebase (I grepped; only the generated files reference it). If it's genuinely part of the QR payload we want to keep round-tripping, a one-line comment saying so would help; otherwise dropping it removes a field readers will keep wondering about.

[deaflynx]

Dropped it. Nothing reads it, and the generated fromJson ignores keys it doesn't know, so a QR link carrying ttl still parses.

[vvlladd28]

This puts platform deep-link plumbing — stream subscription, duplicate suppression, cold-start link consumption — inline in the root widget's build. Could it move behind a small service (or a useAppLinks() hook) that owns the single subscription, so the app root reads as pure composition and there's one authoritative place for the app-link policy?

That would also be the natural home for the tb_context.dart:287 subscription noted in the review body: it's dead today, but two owners of the same stream is a trap worth designing out rather than relying on it staying unreachable.

[deaflynx]

Moved into useAppLinks() (lib/config/routes/use_app_links.dart). The app root is a single call now, and that hook is the one place holding the subscription, the duplicate policy and the cold-start consumption.

The dead TbContext path is deleted too — the subscription, the field and the cancel in logout(). Confirmed unreachable: ThingsboardInitApp and ThingsboardInitRegionApp are never instantiated, so initTbContext() never runs and TbContext.init() can't execute.

[vvlladd28]

The 2-second window is an unexplained magic number, and the mechanism — two mutable closure variables plus a wall-clock diff — is doing a lot of work for what the comment describes as "the platform may deliver the same intent more than once".

If the real problem is the cold-start link arriving from both getInitialAppLink() and the stream, tracking "the last link we actually navigated with" would be more deterministic than a timing window — and note navigateByAppLink already deletes the stored initial link as its first step, so that half may already be covered and the initialLink != lastLink check below may be the only guard you need. At the very least the duration deserves a named constant explaining where 2s comes from.

[deaflynx]

Named the duration (_duplicateLinkWindow) and explained it in the comment, but kept the window rather than tracking only the last link.

Tracking the last link we navigated with would drop a deliberate re-scan of the same QR permanently, and re-scanning the same code inside its TTL is a case this PR tests as logs in again. The window is exactly what separates a doubled platform intent from that. You're right that the cold-start half is already covered by initialLink != lastLink plus navigateByAppLink deleting the stored link — that check stays.

[vvlladd28]

The parenthetical rationale doesn't hold for this particular call, as far as I can tell from the platform source. getLoginMobileInfo maps to GET /api/noauth/mobile (MobileAppController.java:81), which has no @PreAuthorize and sits under /api/noauth/** in NON_TOKEN_BASED_AUTH_ENTRY_POINTS (ThingsboardSecurityConfiguration.java:86) — so it's permitAll. For an unknown pkgName it returns 200 with null storeInfo/versionInfo, not 403 (mobileAppService.findMobileAppByPkgNameAndPlatformType → Optional.ofNullable(…)).

The ignoreErrors marking is harmless and the try/fallback below is still worth having, so this is really about the comment: if you've actually observed a 403 here it'd be good to note which server/version produced it (PE? a reverse proxy? an older release?), otherwise the next reader will go looking for a 403 that the CE code can't emit. The mobile-session calls in notification_service.dart are the ones that genuinely can 403.

[deaflynx]

You're right — I hadn't observed a 403 on this call; the wording was carried over from the mobile-session calls where it does hold. The comment no longer claims one: it now says an unknown package name is answered with an empty payload, and that a proxy or an older server can still fail the request, which is why the marking and the fallback below are still worth keeping.

[vvlladd28]

Review summary

Reviewed 24 changed files in fix: make QR-code login and endpoint switch reliable (PROD-8200). Left 14 comment(s) inline.

The staging → verify → commit restructuring of switchEndpoint is a real improvement, and capturing previousSession before the first write closes the session-loss hole properly. Findings were checked against the server (QrCodeSettingsController, MobileAppSecretServiceImpl, AdminController, ThingsboardErrorResponseHandler) and against the client library, which confirmed two of the PR's own premises: /api/admin/updates really is SYS_ADMIN-only, and the QR secret really is cached without eviction on read, so a re-scan within TTL returns the same already-revoked pair.

The single highest-value item is the opposite of a bug: this PR adds five localized strings and then bypasses them, because server error text wins in _errorMessage and that text is hardcoded English — and misleading ("Token has expired" for a token revoked by logout).

Of the inline comments, 11 are worth acting on here — 8 of them one-liners — and 3 are explicitly marked as follow-up rather than blockers for this PR.

Considered and deliberately not raised

So these read as decided rather than missed: moving the two client storage keys into DatabaseKeys/ILocalDatabaseService (they are the client's keys — relocating them hides the coupling more than it surfaces it); turning NoAuthState into a freezed union; extracting useAppLinks into an IAppLinkService; narrowing ITbClientService.reInit's signature (it touches an interface the PE fork shares, and thingsboard_client.dart documents that CE/PE are merged); reusing a single Dio across the two host calls; renaming fromFluroData and resolveHomePath; and dropping the one-field SwitchEndpointParams wrapper. All defensible, none worth the churn against what this PR is fixing.

---

This review was auto-generated. Findings may contain errors — please verify before applying changes.

[vvlladd28]

This is the one I'd most want changed. The doc on NoAuthState.serverMessage says it is "already localized server-side", but it isn't — the messages are hardcoded English on the server:

// MobileAppSecretServiceImpl.getJwtPair — expired/unknown secret
throw new ThingsboardException("Jwt token not found or expired!", JWT_TOKEN_EXPIRED);

// ThingsboardErrorResponseHandler:321 — what /api/auth/user returns for a revoked token
ThingsboardErrorResponse.of("Token has expired", JWT_TOKEN_EXPIRED, HttpStatus.UNAUTHORIZED)

Two consequences. A user on the German or Spanish build gets untranslated API-consumer text. And for the headline case this PR fixes — re-scanning a QR after a logout — the server says "Token has expired", which is actively wrong: the token isn't expired, it was revoked by the logout watermark. Meanwhile qrCodeSessionIsNoLongerValid ("The QR code session is no longer valid. Please refresh the QR code and scan again.") is both accurate and localized, and because serverMessage wins here it is effectively unreachable.

Suggest inverting the priority: use the localized copy for the kinds you already classify, and fall back to serverMessage only for NoAuthFailure.unknown.

return switch (state.failure) {
  NoAuthFailure.tokenExchangeFailed =>
    S.of(context).failedToObtainLoginTokenFromHost(state.host ?? ''),
  NoAuthFailure.sessionInvalid => S.of(context).qrCodeSessionIsNoLongerValid,
  NoAuthFailure.unknown || null =>
    (serverMessage != null && serverMessage.isNotEmpty)
        ? serverMessage
        : S.of(context).somethingWentWrong,
};

The comment on the field should stop claiming server messages are localized either way.

[deaflynx]

Inverted in 5579b78: tokenExchangeFailed and sessionInvalid always use the localized copy, and serverMessage is consulted only for NoAuthFailure.unknown. The docs on serverMessage and NoAuthFailure no longer claim server text is localized.

[vvlladd28]

Lines 67–74 sit outside the try, and the call site doesn't await or catch:

// switch_endpoint_noauth_view.dart:36
ref.read(noauthProviderProvider.notifier)
   .switchEndpoint(SwitchEndpointParams(data: arguments));

So anything that throws in that block becomes an unhandled zone error and the view sits on the spinner forever — the failure mode this PR removes everywhere else. There are three throwing expressions in it, and I checked them in Dart:

tb:/api/noauth/qr?secret=x | isAbsolute=true  path=/api/noauth/qr  origin=THROWS StateError
Uri.parse('http://[bad')                                          THROWS FormatException

Uri.isAbsolute only means "has a scheme and no fragment" — it does not imply http/https, and note the path still matches this GoRouter route, so such a link does reach here. onLoginWithBarcode (login_widget.dart:219) hands any scanned barcode to navigateByAppLink, so this is reachable, though I'll be honest that it's exotic: the server only ever generates https:// links (QrCodeSettingsController.DEEP_LINK_PATTERN), so a legitimate QR won't hit it.

I'd still fix it, because the fix is free and the invariant is the point rather than the exotic input — move the block inside the try (keeping previousEndpoint available to the catch), and make the guard say what it means:

final host = params.data.host ??
    ((uri.isScheme('http') || uri.isScheme('https')) ? uri.origin : previousEndpoint);

That turns the worst case into the normal error screen with a rollback instead of a hang.

[deaflynx]

Fixed in 5579b78: the host is resolved inside the try via _resolveHost, guarded on isScheme('http') || isScheme('https'). A malformed link (non-http scheme, empty host) now ends on the regular error screen with rollback. _failureState takes a nullable host for the case where resolution itself is what failed.

[vvlladd28]

The onError handler that the deleted TbContext code had didn't come across:

// removed from tb_context.dart
appLinks.uriLinkStream.listen(
  (link) { ... },
  onError: (err) => log.error('linkStream.listen $err'),
);

App links are the entry point for the whole QR flow, so a platform-side stream error now becomes an unhandled zone error instead of something diagnosable in the logs. Worth carrying the handler over — one line.

[deaflynx]

Added in 5579b78: onError on the stream logs through TbLogger.

[vvlladd28]

I checked the premise and it holds — /api/admin/updates really is @PreAuthorize("hasAuthority('SYS_ADMIN')") (AdminController.java:434), so every non-sysadmin login genuinely gets a 403 here. Given the client library can't be changed from this PR, a time window is a reasonable workaround and the counter is the right call.

The one thing I'd tighten: this mutes the app's entire client error channel for those 2 s, and in the QR flow that's load-bearing. reInit is immediately followed by loadUser(), so if the server is unreachable at that moment the toast is dropped, the user watches a spinner, and 12 s later _userLoadTimeout drops them on the login page with no explanation at all.

Keeping the most important error class visible is one line:

if (_suppressErrorNotifications && !Utils.isConnectionError(e)) {
  return;
}

Utils.isConnectionError (utils.dart:309) matches exactly the "Unable to connect" case, which is never what the init-time 403 looks like — so this doesn't weaken the suppression you're after.

[deaflynx]

Applied in 5579b78: _suppressErrorNotifications && !Utils.isConnectionError(e). Checked the client: init() swallows the version-check transport failure, so without this the reInit path really did complete silently and the 12 s fallback was the only feedback.

[vvlladd28]

error is never read for its value — only != null, at switch_endpoint_noauth_view.dart:55 and :99. The actual message comes from failure/serverMessage, which is the right design. Since failure is non-null exactly when error is, the field can just go and the two checks become state.failure != null.

If you want to keep the original exception for logging, _logger.error(...) in the catch already has it, so it doesn't need to live in the state.

[deaflynx]

Removed in 5579b78. Both view checks use failure != null; the exception is still logged in the catch.

[vvlladd28]

Nice tightening. While you're in this file: the commented-out init() block just above (lines 18–29) references the exact code this replaced (t.bottomBarPages.first), so it now reads as a live alternative to freshly changed logic — the kind of thing that gets resurrected by mistake. Worth deleting; git has it if anyone needs it.

[deaflynx]

Deleted in 5579b78, together with three imports in the same file that were already unused.

[vvlladd28]

Small factual fix to the comment: it says "the client pinned in pubspec.yaml", but thingsboard_ce_client is a path: dependency on ../thingsboard-dart-client/ce — there's no pin, which makes the drift risk a bit higher than the comment implies. Worth saying "the client in ../thingsboard-dart-client/ce" instead, so the next reader knows there's no version to check against.

I did verify the keys themselves are right (_setUserFromJwtToken / init() both use 'jwt_token' and 'refresh_token'), and I'd leave them here as named constants rather than moving them into DatabaseKeys — they're the client's keys, not the app's, and relocating them into the app's own key file would hide the coupling more than surface it.

[deaflynx]

Reworded in 5579b78: the comment points at ../thingsboard-dart-client/ce and says there is no version pin to check against.

[vvlladd28]

Not asking for a change in this PR — flagging it for the client-library follow-up you already list under "Still open", because it's the same root cause.

The comment is honest that this covers losing the race with the outgoing client, but the check is a single snapshot taken right after reInit, and nothing stops the old client afterwards. Traced it through the client:

// refreshJwtToken
} catch (e) {
  await _clearJwtToken();   // -> _setUserFromJwtToken(null, null, true)
  rethrow;                  //    -> storage.deleteItem('jwt_token' / 'refresh_token')
}

The old client holds the same TbStorage singleton, so an in-flight request that 401s after this guard has passed wipes the freshly staged pair. The new client already has the tokens in memory, so the running session looks fine — it's the persisted copy that's gone, and the user finds themselves logged out on the next cold start. Silent, and indistinguishable from the original PROD-8200 report.

A real fix means being able to quiesce the outgoing client (a CancelToken on its Dio, or a dispose() that stops it writing to storage), which belongs in the same client change as the notify && !config.ignoreErrors fix. An app-side workaround here would just be another timing guess.

[deaflynx]

Agreed, leaving this for the client-library follow-up: quiescing the outgoing client belongs next to the notify && !config.ignoreErrors fix.

[vvlladd28]

Low priority, and I'd understand deferring it. This compares only Uri.host, so scheme and port are ignored: http://acme.local:8080 and https://acme.local:8443 count as the same host. That was mostly harmless before, but the new no-secret branch keys off it —

if (!isTheSameHost) {
  await _switchHostOnly(host: host, previousEndpoint: previousEndpoint);
}
state = const NoAuthState(isDone: true);

— so a host-only link differing only by port or scheme performs no switch at all and silently lands the user on the host they were already on. The server's host= parameter is a full baseUrl (QrCodeSettingsController.getMobileAppDeepLink), and EndpointService stores endpoints as full URLs including scheme, so the two really can differ that way on self-hosted HTTP deployments.

Uri.parse(host).origin == Uri.parse(previousEndpoint).origin would fix it in one line, but it also changes cross-scheme switches into a full endpoint + Firebase re-init, and there are no tests to catch fallout — so this seems reasonable to schedule rather than squeeze in.

[deaflynx]

Deferring as suggested. The one-liner also turns cross-scheme switches into a full endpoint + Firebase re-init, and nothing would catch fallout yet, so it goes with the tests follow-up.

[deaflynx]

Reconsidered and included after all in a2e175c: isTheSameHost is now isSameOrigin, comparing Uri.origin of the target and the previous endpoint. Checked in Dart that origin normalizes case, default ports and paths, so https://Acme.local:443/ still matches https://acme.local. A host-only link differing only by scheme or port now performs the full switch; a secret link in the same situation swaps the Firebase apps, where isCustomEndpoint still compares hosts on purpose (Firebase is bound to the default host). A schemeless host= value now throws inside the try and ends on the error screen instead of being stored as an unusable endpoint. Analyzer is clean; the scheme/port switch cases still need a device run, since the repo has no tests yet.

[vvlladd28]

Agreed with your "Still open" note, and not asking for tests in this PR — but I'd rather it became a ticket than an assumption that the flow is untestable, because I don't think it is.

Everything this method touches is registered in locator.dart behind an interface as a lazy singleton (IEndpointService, ITbClientService, IFirebaseService, TbStorage), and mocktail, flutter_test and integration_test are already in dev_dependencies. A setUp doing getIt.reset() plus fakes, driving the notifier through a ProviderContainer, would reach every branch that regressed before: host-only vs. secret switch, same-host vs. cross-host, exchange failure, verify failure, and the rollback restoring both endpoint and session.

The minimal seam if you'd rather not lean on getIt.reset() is resolving those four collaborators once in build() into fields, so a test can override them in one place — cheap to do now, and it's what makes the tests possible later.

Worth acknowledging the repo has no test/ directory at all, so this would be the first one. That's a real cost and a fair reason to split it out — this flow just seems like the right place to spend it, given it can silently log a user out.

[deaflynx]

Not in this PR, but agreed it is testable. The switch flow (host-only vs secret, same vs cross host, exchange/verify failure, rollback restoring endpoint and session) is the candidate for the first test/, with the collaborators resolved once in build() as the seam.

[vvlladd28]

Re-review summary

Re-reviewed fix: make QR-code login and endpoint switch reliable (PROD-8200) — verified 14 finding(s) from previous review, covering the two commits pushed since (5579b78, a2e175c).

Status	Count
✅ Resolved	12
💬 Acknowledged	2
❌ Unresolved	0

Every finding was addressed, and the fixes are the ones that were asked for rather than the cheapest thing that would close the comment. Three claims from the replies were spot-checked against the sources and hold: InterceptorConfig.toExtra() emits the same flat ignoreErrors/ignoreLoading keys the old raw map used, so the 2FA migration is behaviour-preserving apart from the acknowledged ignoreLoading; the platform stop() early-returns on a null _textureId, so dispose() does not double-stop the scanner; and MobileAppSecretServiceImpl.getJwtPair does confirm the re-scan premise — the secret is read from cache with no eviction, so a re-scan inside the TTL returns the same already-revoked pair.

Found 4 new issue(s) in the fix commits, commented inline. Only the first is substantive; the other three are nits you can take or leave.

The one worth acting on is the failure classification: _verifySession raises sessionInvalid for any DioException, so a connect timeout or a 502 from a proxy in front of the target host now tells the user "The QR code session is no longer valid. Please refresh the QR code and scan again." Checked against the server — JWT_TOKEN_EXPIRED maps to 401 (ThingsboardErrorResponseHandler:111) and /api/auth/user answers exactly that for a revoked pair (:321) — so a 401 check separates the two cases exactly, not heuristically. SwitchEndpointFailure.status is already captured for this and currently read by nothing.

Considered and deliberately not raised

Four more things came up and were dropped after checking them, so they read as decided rather than missed:

• a2e175c makes _isSameOrigin throw where the old Uri.host returned '' — real (Uri.parse('').origin throws StateError on the pinned Dart 3.29.0), but the reachable version of it is narrow, and the intended half of it is an improvement. See the inline comment — it asks only for the asymmetric guard, not a rollback of the change.
• A dedicated NoAuthFailure.invalidLink for malformed links. The server's host= is systemSecurityService.getBaseUrl(...), which can be free text an admin typed, so a schemeless value is possible — but the new behaviour (clean error + rollback with the session preserved) is already better than the old one (store acme.local, break later). A new enum value plus an ARB key plus 13 locales falling back to English isn't worth it over somethingWentWrong when the catch already logs the detail.
• The mutable String? host local in switchEndpoint. Every alternative that keeps it final came out longer than what's there.
• unawaited(controller.stop()) not handling a rejected invokeMethod. Technically true — unawaited silences the lint, not the error — but the same file already has two awaited controller.stop() calls with no try/catch (lines 68 and 169), so singling this one out would be inconsistent rather than safer.

Also still agreed as out of scope: the client-side notify && !config.ignoreErrors fix, and tests for the switch flow. If the client PR does happen, exporting the two storage key names as constants from thingsboard_client_base.dart would be a cheap thing to fold into it — it would turn the documented "must be kept in sync" here into a compile error. A stageSession() API for the same purpose would be overkill.

Finding details

• ✅ lib/core/auth/noauth/presentation/view/switch_endpoint_noauth_view.dart:181 — hardcoded-English server text won over the five new localized strings — Fixed in code: serverMessage is now consulted only for NoAuthFailure.unknown, and the docs no longer claim server text is localized.
• ✅ lib/core/auth/noauth/provider/noauth_provider.dart:77 — host resolution sat outside the try, so a malformed link hung the view on the spinner — Fixed in code: _resolveHost runs inside the try and guards on isScheme('http') || isScheme('https') instead of isAbsolute.
• ✅ lib/config/routes/use_app_links.dart:44 — the deleted TbContext code's onError handler didn't come across — Fixed in code: stream errors are logged through TbLogger.
• ✅ lib/utils/services/tb_client_service/tb_client_service.dart:106 — the init-time window muted the whole client error channel, including "Unable to connect" — Fixed in code: _suppressErrorNotifications && !Utils.isConnectionError(e).
• ✅ lib/core/auth/noauth/provider/noauth_provider.dart:343 — NoAuthState.error was never read for its value — Fixed in code: field removed, both view checks use failure != null, the exception is still logged in the catch.
• ✅ lib/core/auth/noauth/provider/noauth_provider.dart:192 — _installSession widened _ExchangedSession back to _Session — Fixed in code: the parameter is _ExchangedSession, so _Session now means only the stored pair.
• ✅ lib/config/routes/use_app_links.dart:39 — refreshing lastLinkAt on a dropped delivery made the fixed window slide, contradicting the doc comment — Fixed in code: the refresh is gone, behaviour matches the comment.
• ✅ lib/utils/best_effort_request.dart:12 — two spellings of the same Dio flag — Fixed in code: both 2FA call sites migrated and ThingsboardAppConstants.ignoreErrors deleted; confirmed no references remain.
• ✅ lib/utils/ui/qr_code_scanner/qr_code_scanner.dart:102 — the decoder kept running at full frame rate after the result was in hand — Fixed in code: unawaited(controller.stop()) alongside the latch. (Small correction to the reply: pubspec.lock resolves mobile_scanner to 7.1.3, not 7.2.0 — same code path.)
• ✅ lib/config/routes/v2/route_handlers/home_handler.dart:13 — a commented-out init() block read as a live alternative to freshly changed logic — Fixed in code: deleted, along with three imports that became unused; the remaining imports all still resolve.
• ✅ lib/core/auth/noauth/provider/noauth_provider.dart:17 — the comment claimed the client was pinned in pubspec.yaml — Fixed in code: it now points at ../thingsboard-dart-client/ce and says there is no version pin.
• 💬 lib/core/auth/noauth/provider/noauth_provider.dart:205 — the outgoing client's failing refresh can still wipe the freshly staged pair — Developer: "agreed, leaving this for the client-library follow-up: quiescing the outgoing client belongs next to the notify && !config.ignoreErrors fix." Agreed — this was flagged as follow-up rather than a blocker, and the app-side snapshot check is the right stopgap.
• ✅ lib/core/auth/noauth/provider/noauth_provider.dart:135 — the comparison ignored scheme and port, so a host-only link differing only there performed no switch — Fixed in code: _isSameOrigin compares Uri.origin. Offered as deferrable and included anyway; see the inline comment for the one edge it introduces.
• 💬 lib/core/auth/noauth/provider/noauth_provider.dart:66 — no tests for the switch flow — Developer: "not in this PR, but agreed it is testable", with the collaborators resolved once in build() named as the seam. Reasonable — tests were explicitly not requested here.

---

This re-review was auto-generated. Findings may contain errors — please verify before applying changes.

[vvlladd28]

This raises sessionInvalid for any DioException, and now that the localized copy always wins in the view, that arm is unconditional — so a connect/receive timeout against the target host, or a 502 from a proxy in front of it, tells the user "The QR code session is no longer valid. Please refresh the QR code and scan again." Refreshing the QR won't help with any of those, and before the inversion a server-supplied message at least had a chance to say something truer.

The good news is that the split is exact rather than heuristic. Checked on the server side:

// ThingsboardErrorResponseHandler:111
errorCodeToStatusMap.put(ThingsboardErrorCode.JWT_TOKEN_EXPIRED, HttpStatus.UNAUTHORIZED);
// :321 — what /api/auth/user answers for a revoked pair
ThingsboardErrorResponse.of("Token has expired", JWT_TOKEN_EXPIRED, HttpStatus.UNAUTHORIZED)

So "the session really is invalid" is always a 401, and everything else is something the QR copy shouldn't be claiming. _asFailure already captures the status for exactly this purpose and nothing reads it today:

} on DioException catch (e) {
  throw _asFailure(
    e,
    e.response?.statusCode == 401
        ? NoAuthFailure.sessionInvalid
        : NoAuthFailure.unknown,
  );
}

Non-401s then fall through to the server's message, or somethingWentWrong when there isn't one — which is the right answer for an unreachable host.

One related nit while you're here: the new comment at switch_endpoint_noauth_view.dart:190 cites "Token has expired" as the reason to distrust serverMessage, but that response is a 401 and is therefore classified sessionInvalid — it can never reach the unknown arm the comment annotates. Moving that half of the note up to the sessionInvalid case (where it explains why the localized string exists) would keep it accurate.

[deaflynx]

Fixed in c05f5e7: _verifySession maps a 401 to sessionInvalid and everything else to unknown, so a timeout or a proxy error falls through to the server message or somethingWentWrong. The "Token has expired" note in the view moved up to the sessionInvalid arm, where that response actually lands.

[vvlladd28]

Worth a small guard, though not a rollback of the change — most of what this does is an improvement.

Uri.origin throws where Uri.host quietly returned '', verified on the pinned Dart (3.29.0):

Uri.parse('').origin                     -> THROWS StateError: Cannot use origin without a scheme
Uri.parse('demo.thingsboard.io').origin  -> THROWS StateError
OLD: Uri.parse('').host                  -> ''

For the target host that's exactly what you intended, and I agree with it: I checked the server, and host= is systemSecurityService.getBaseUrl(...), which resolves either to MiscUtils.constructBaseUrl ("%s://%s:%d", always scheme-qualified — and Uri.origin normalizes the explicit :443 away, so that case still compares equal) or to a free-text baseUrl an admin typed into general settings / white labeling, with no scheme validation I could find. A schemeless value there used to be stored verbatim as the endpoint and break later; failing fast with a rollback that preserves the session is strictly better.

The asymmetry is the part I'd change: previousEndpoint gets the same strictness, and there it buys nothing — if the stored endpoint is empty or unparseable there's nothing to stay on, so the switch should just proceed rather than abort. getEndpoint() has an explicit ?? ThingsboardAppConstants.thingsBoardApiEndpoint fallback that is '' when the dart-define isn't passed, and an app upgraded from a build that accepted a schemeless host= can also be carrying one in storage.

bool _isSameOrigin(String host, String previousEndpoint) {
  final target = Uri.parse(host).origin; // deliberately throws on an unusable target
  final previous = Uri.tryParse(previousEndpoint);
  if (previous == null ||
      !(previous.isScheme('http') || previous.isScheme('https')) ||
      previous.host.isEmpty) {
    return false; // nothing to compare against — treat as a different server
  }
  return target == previous.origin;
}

Note a symmetric tryParse on both sides would be the wrong fix — it would swallow the bad-target case you added on purpose.

While here: other doesn't say anything, and naming the parameters for their roles (as above) also documents which side is allowed to be missing. And EndpointService.isCustomEndpoint still compares Uri.parse(...).host only, so "same server" now means two different things in two files — deliberate for Firebase per your reply, but worth a line saying so, or the next change will unify them by accident.

[deaflynx]

Guarded in c05f5e7 with the asymmetric shape: the target still goes through Uri.parse(host).origin and throws on an unusable value, while the previous endpoint goes through Uri.tryParse and counts as a different server when it is missing, non-http or has an empty host. Parameters are now host / previousEndpoint, the scheme check is shared with _resolveHost via _isHttpUri, and the doc says why EndpointService.isCustomEndpoint keeps comparing hosts only (Firebase is bound to the default host).

[vvlladd28]

Nit, take it or leave it. The carve-out is right, but the getter no longer answers the question being asked at the call site: _suppressErrorNotifications reads as "suppress", while the actual rule is "suppress unless it's a connection error" — the reader has to hold the negation and the comment together, and Utils.isConnectionError(e) appears again four lines below.

Folding it into a _shouldSuppress(e) predicate that owns the exception would let the guard read as one intent. (To be clear about the second call: isConnectionError compares three fields, so this is about readability, not cost.)

[deaflynx]

Folded in c05f5e7: _shouldSuppress(e) owns both the pending-init window and the connection-error carve-out, so onClientError reads as one intent.

[vvlladd28]

Nit, and I can see the argument for leaving it. Migrating the 2FA sites onto this helper removed the duplicate flag, which was the point — but it stretches the helper past what its name promises. A "best-effort" request is one whose failure nobody is told about, and verifyCode does tell them:

// two_factor_confirm_provider.dart
if (e.status == 429) {
  state = state.copyWith(codeState: CodeState.tooManyRequests, loading: false);
  return;
}
state = state.copyWith(codeState: CodeState.invalid);

So the doc comment now has to carve out an exception for those two callers, which is usually the sign of one name covering two intents — suppressGlobalErrorExtra() would describe what the flag actually does at every call site. Against that: it's ~5 call sites here plus the mirrored ones in the PE fork, and the carve-out in the doc does make the current state unambiguous. Your call.

The bundled ignoreLoading: true is worth a word either way — I confirmed it's harmless because the 2FA provider drives its own state.loading, but a reader at the call site has no way to know bestEffortRequestExtra() also opted them out of the global loading indicator.

[deaflynx]

Renamed in c05f5e7, with a different name than suggested: silentRequestExtra() in lib/utils/silent_request.dart. suppressGlobalErrorExtra would have covered only half of what the flag does, since it also opts the call out of the global loading indicator; the doc now states both effects up front and lists the two groups of callers without a carve-out. Nine call sites across three files updated, no references to the old name remain.

[vvlladd28]

Re-review summary

Re-reviewed fix: make QR-code login and endpoint switch reliable (PROD-8200) — verified 4 finding(s) from previous review, covering the one commit pushed since (c05f5e7).

Status	Count
✅ Resolved	4
💬 Acknowledged	0
❌ Unresolved	0

All four are properly fixed, and two came back better than what was asked for.

The _isSameOrigin guard was verified empirically against the pinned Dart (3.29.0) rather than read: an empty, schemeless, custom-scheme, host-less or unparseable stored endpoint all return false and let the switch proceed, while acme.local or '' as the target still throws — the asymmetry that was wanted. The guard's three conditions line up exactly with the three cases Uri.origin throws on, so there is no gap between the check and what it protects, and Uri.tryParse returning null is a real case (http://[bad) that is handled.

silentRequestExtra() is a better answer than the suppressGlobalErrorExtra() that was suggested — it covers the loading half too, and the new doc leads with both effects instead of carving out an exception for the 2FA callers. The migration is complete: 9 call sites across three files, no references to the old name or to _suppressErrorNotifications remain.

Found 2 new issue(s) in the fix commit, commented inline. Neither blocks the merge.

One correction to the previous round that changes where the remaining value is. _verifySession only runs once _exchangeSecret has already been answered by the same host over the same Dio, so an unreachable host or a connect timeout always surfaces one method earlier and never reaches the call that was just fixed. The 401 narrowing in _verifySession is still right — it fixes the 5xx and 403 answers, and a proxy failing between the two calls is possible — but the timeout case, which was the motivating example, lives in _exchangeSecret and is untouched. The inline comment reframes what's worth doing about that: the copy fix matters more than the classification, and it improves both call sites.

One note outside the code: the PR description's follow-up section still documents only d57ab30, while three rounds have landed since (5579b78, a2e175c, c05f5e7). The "Still open" list itself is accurate and current.

Finding details

• ✅ lib/core/auth/noauth/provider/noauth_provider.dart:202 — _verifySession raised sessionInvalid for any DioException, so a timeout or proxy error told the user to re-scan the QR — Fixed in code: a 401 maps to sessionInvalid, everything else to unknown. The "Token has expired" note moved up to the sessionInvalid arm in the view, where that response actually lands.
• ✅ lib/core/auth/noauth/provider/noauth_provider.dart:140 — Uri.origin threw on a stored endpoint where Uri.host returned '' — Fixed in code with the asymmetric shape: the target keeps Uri.parse(host).origin, the previous endpoint goes through Uri.tryParse plus scheme and host checks and counts as a different server when unusable. Parameters renamed, _isHttpUri shared with _resolveHost, and the doc explains the isCustomEndpoint divergence.
• ✅ lib/utils/services/tb_client_service/tb_client_service.dart:39 — _suppressErrorNotifications read as "suppress" while the rule was "suppress unless connection error" — Fixed in code: _shouldSuppress(e) owns both conditions and onClientError reads as one intent.
• ✅ lib/utils/silent_request.dart:14 — bestEffortRequestExtra() was stretched over the 2FA callers, which do report failure inline — Fixed in code, and named better than suggested: silentRequestExtra() in lib/utils/silent_request.dart, documenting the error and loading halves up front.

---

This re-review was auto-generated. Findings may contain errors — please verify before applying changes.

[vvlladd28]

This is where the timeout case you just fixed in _verifySession actually lands. _verifySession only runs after _exchangeSecret has already been answered by the same host over the same _hostClient, so reaching it proves the host is up — an unreachable host or a connect timeout always fails here, one method earlier, and every DioException here still becomes tokenExchangeFailed:

"failedToObtainLoginTokenFromHost": "Failed to obtain a login token from {host}. Please scan a new QR code."

So after a 20 s _hostRequestTimeout the user is told to scan a new QR code, which is exactly the misdirection the _verifySession change removes.

The copy is the more valuable half, so I'd start there rather than with the classification. Narrowing this site on 401 alone would only trade a misleading message for a vaguer one — a transport failure has no response body, so it lands on unknown with no serverMessage and renders a bare "Something Went Wrong". The app already has the right strings, used by TbClientService.onClientError:

"connectionError": "Connection error",
"failedToConnectToServer": "Failed to connect to server",

A NoAuthFailure.connectionFailed arm needs no new l10n keys, and it improves both call sites — _verifySession's new unknown branch has the same bare-fallback problem for the rarer proxy case.

Detection is simpler than enumerating DioExceptionType: e.response == null means no HTTP answer came back at all, which is precisely the transport failure. That collapses both changes into one shared classifier:

NoAuthFailure _classify(DioException e, NoAuthFailure whenRejected) =>
    e.response == null
        ? NoAuthFailure.connectionFailed
        : e.response!.statusCode == 401
            ? whenRejected
            : NoAuthFailure.unknown;

_asFailure is the natural home for it — it already reads e.response?.statusCode for the status field, so the policy would live in one place instead of _verifySession inspecting the status inline and this site not inspecting it at all.

On the 401 half being exact here as well: /api/noauth/** is in NON_TOKEN_BASED_AUTH_ENTRY_POINTS and its permitAll() is evaluated before TOKEN_BASED_AUTH_ENTRY_POINTS.authenticated() (ThingsboardSecurityConfiguration:162, :397, :406), so a 401 from /api/noauth/qr/{secret} can only come from MobileAppSecretServiceImpl.getJwtPair throwing JWT_TOKEN_EXPIRED — there's no "unauthenticated" 401 to confuse it with. 401 keeps tokenExchangeFailed, whose host-named copy is right for a stale secret.

Entirely reasonable as a follow-up if you'd rather not grow this PR further — but I'd take the two halves together or neither, since the classification on its own doesn't leave the user better off.

Related, and cheap either way: SwitchEndpointFailure.status is still read nowhere except toString(). Whatever ends up doing the classifying should either use it or the field should say it's diagnostics-only.

[deaflynx]

Took both halves in 6db5d02. New NoAuthFailure.connectionFailed, rendered with the existing failedToConnectToServer string, so no l10n keys were added. The policy lives once in _asFailure: no response is connectionFailed, a 401 becomes the caller's rejection kind (tokenExchangeFailed or sessionInvalid), any other status is unknown so the server's message can speak. Both call sites now pass only their rejection kind and the inline 401 check from c05f5e7 is gone. SwitchEndpointFailure.status is documented as diagnostics only, and the exchange site notes why a 401 there can only be a rejected secret.

[vvlladd28]

Naming nit on an otherwise exactly-right fix. previousEndpoint is a clear improvement over other, but it leaves the first parameter as the odd one out: host holds a full URL, not a hostname — _resolveHost returns args.host (the server's baseUrl, which for an admin-entered value can even carry a path) or uri.origin or the stored endpoint. Two lines down, previous.host uses the same word in the narrower sense. Both meanings inside one eight-line method costs a reader a second pass; targetEndpoint would pair with previousEndpoint and match the target / previous locals already there.

[deaflynx]

Renamed in 6db5d02: _isSameOrigin(String targetEndpoint, String previousEndpoint), pairing with the target / previous locals.

[vvlladd28]

Re-review summary

Re-reviewed fix: make QR-code login and endpoint switch reliable (PROD-8200) — verified 2 finding(s) from previous review, covering the one commit pushed since (6db5d02).

Status	Count
✅ Resolved	2
💬 Acknowledged	0
❌ Unresolved	0

Both are fixed, and the first took the shape that was asked for: the classification moved into _asFailure rather than being duplicated at a second call site, the inline 401 check that c05f5e7 had added to _verifySession is gone, and connectionFailed reuses failedToConnectToServer so no .arb key was added.

The load-bearing claim of this commit checks out on both editions. _exchangeSecret now sends every non-401 answer to unknown, which gives up the host-named "Please scan a new QR code" copy for anything that isn't a 401 — so the new comment ("a 401 here can only be the secret being rejected") only holds if a rejected secret is always a 401. It is: MobileAppSecretServiceImpl.getJwtPair has exactly one throw, ThingsboardErrorCode.JWT_TOKEN_EXPIRED for a cache miss (expired or unknown secret), mapped to UNAUTHORIZED by ThingsboardErrorResponseHandler (:92 in CE, :111 in PE; the service is byte-identical between editions). The only other throw reachable on /api/noauth/qr/{secret} is checkParameter's 400 for a blank secret, which this flow can't produce — switchEndpoint short-circuits an empty secret into the host-only branch before the call, and an empty @PathVariable wouldn't match the mapping anyway.

One PE case makes the change strictly better rather than merely neutral. /api/noauth/qr/** falls inside SETUP_PROTECTED_ENTRY_POINTS (/api/**) and is absent from SETUP_ALLOWED_ENTRY_POINTS, so while an instance is unprovisioned or re-locked, SystemSetupFilter answers it with SETUP_INCOMPLETE → HttpStatus.LOCKED (423) carrying a JSON message of "System setup is not complete". Before this commit that became tokenExchangeFailed and told the user to scan a new QR code; now it lands on unknown and shows what the server actually said. Worth carrying over to the PE-side PR.

_verifySession is a strict refinement of what c05f5e7 did: 401 still means sessionInvalid, other statuses still mean unknown, and only the no-response case changed. The view's switch stays exhaustive over the enum, and nothing anywhere reads NoAuthFailure by index or .values, so inserting connectionFailed first is safe.

Found 3 new issue(s) in the fix commit, commented inline, ordered by value: the copy one is worth taking, the diagnostics one is worth taking cheaply, the third is a nit.

The client-library item is confirmed, and no longer reachable from the path this PR fixes

Checked the pinned client rather than taking the description's word for it, since this is the one open item that touches the bug itself. Both halves are exactly as described:

• http_interceptor.dart:145 honours notify && !ignoreErrors on the ordinary error path, but :157 — the branch taken after a failed refresh — passes notify as a hardcoded true, so ignoreErrors cannot suppress that toast.
• the session wipe is one level down: thingsboard_client_base.dart:334 calls _clearJwtToken() when the refresh request throws, and :338 does the same when the stored refresh token is already invalid.

So a best-effort call can still both toast and clear the staged session, and silentRequestExtra() is right to document that it only covers the UI. What has changed is that the dangerous instance of it is gone: the original failure needed a stale client aimed at the old host holding the new host's tokens, which guaranteed the 401 that started the chain. Reading the live client through a getter, staging the pair before reInit, and verifying it against the target host all remove that guarantee. What remains is genuine expiry or revocation — where clearing the session is the correct outcome — plus a spurious toast. That matches the decision to leave it to a client PR.

One note outside the code

Nothing has mechanically verified this branch, and it's worth being explicit about that given the surface area. pubspec.yaml on develop/1.9.0 — not something this PR touches — declares thingsboard_ce_client as path: ../thingsboard-dart-client/ce, so the repository can only be resolved next to a sibling checkout of the client. This PR reports no status checks at all, and a fresh worktree of the branch can't even run flutter analyze without that layout, so the analyze result in the description and the emulator/device matrix are the only evidence this branch has. Combined with the absence of a test/ directory, that puts the whole weight of a 820/269-line change in the authentication path on manual verification. Not a finding against this PR, and not something to fix here — but it is the largest remaining risk, and it argues for the tests follow-up being scheduled rather than open-ended.

The PR description's follow-up section now documents all four rounds, which closes the one out-of-code note from the last review.

Finding details

• ✅ lib/core/auth/noauth/provider/noauth_provider.dart:361 — every DioException in _exchangeSecret became tokenExchangeFailed, so a 20 s timeout told the user to scan a new QR code; SwitchEndpointFailure.status was read nowhere — Fixed in code, both halves. _asFailure owns the policy (no response → connectionFailed, 401 → the caller's rejection kind, anything else → unknown), both call sites pass only their rejection kind, and status is documented as diagnostics-only.
• ✅ lib/core/auth/noauth/provider/noauth_provider.dart:149 — _isSameOrigin's first parameter was named host while holding a full URL, colliding with previous.host two lines below — Fixed in code: _isSameOrigin(String targetEndpoint, String previousEndpoint), pairing with the target / previous locals.

---

This re-review was auto-generated. Findings may contain errors — please verify before applying changes.

[vvlladd28]

Reusing the existing key is what kept this commit free of new .arb entries, but it's worth weighing that against what the user sees: this is now the only arm of the switch that doesn't name the host, and it's the arm where the host is the single most useful thing to show — an unreachable, mistyped or wrong-port QR target. "Failed to connect to server" gives the user nothing to act on; "Failed to connect to {host}" tells them the QR pointed somewhere they can't reach.

The cost is smaller than it looks. This PR already added five keys to intl_en.arb only — none of them exist in the other twelve .arb files, which fall back to English — so a failedToConnectToHost key is one line plus a regen, the same thing you did four times already, and {host}-parameterized copy is the established shape here (failedToObtainLoginTokenFromHost, gettingDataFromHost, loggingYouIntoHost, switchingToNewHost).

state.host is also guaranteed non-null on exactly this path: connectionFailed can only come from a DioException raised by _exchangeSecret or _verifySession, both of which run after _resolveHost has assigned host — a _resolveHost failure is a Uri error and lands on unknown instead. So the new string can take a non-empty host without a ?? '' guard.

[deaflynx]

Added in 272d62b: failedToConnectToHost ("Failed to connect to {host}") in intl_en.arb, regenerated, and the connectionFailed arm uses it. Kept the ?? '' guard only because state.host stays typed String?; agreed it is never null on this path.

[vvlladd28]

This branch drops the one thing that distinguishes the transport failures from each other. switchEndpoint's catch logs the SwitchEndpointFailure, not the DioException, and nothing else logs it, so a failure here leaves only connectionFailed behind — no way to separate a connect timeout from a receive timeout, a refused connection, a DNS failure or a bad certificate. A self-signed cert on a self-hosted instance is the case I'd care about most: badCertificate is a configuration problem with an obvious fix, and right now it is indistinguishable from "the server is down". Pre-existing rather than a regression — the old code logged an equally opaque tokenExchangeFailed — but this commit is what makes the branch explicit, so it's the natural moment.

The proportionate fix is one field, not the whole exception: SwitchEndpointFailure already carries status for diagnostics only, so a final DioExceptionType? cause alongside it is symmetric, keeps _asFailure free of side effects, and the existing catch log picks it up through toString() with no new log call. Carrying the whole DioException would be the overweight version of the same idea, and a _logger.error inside _asFailure would work too but trades the method's purity for it.

Which matters because of the second half: _asFailure is now the single decision point for how every network failure in this flow is presented, with four distinct outcomes, and it touches no instance state at all. The PR description's tests blocker (getIt collaborators, inline Dio) doesn't actually apply to it — as a top-level function in this library it would be unit-testable today, with a hand-built DioException and no injection work. Worth knowing when that item gets picked up, since it's the highest-value thing in the file to pin down and the cheapest to reach.

[deaflynx]

Added in 272d62b: SwitchEndpointFailure.cause (DioExceptionType?), filled from e.type on both branches of _asFailure and printed by toString(), so the existing catch log now separates a timeout from a refused connection or a bad certificate. No new log call, _asFailure stays side-effect free. Noted the point about it being unit-testable on its own for the tests follow-up.

[vvlladd28]

Nit, and the least of the three — skip it if you'd rather not respin. The call sites read _asFailure(e, NoAuthFailure.tokenExchangeFailed), which looks like "classify this as tokenExchangeFailed"; you have to come back up to the doc comment to learn the argument applies only on a 401 and that everything else lands on unknown. _asFailure(e, rejectedAs: NoAuthFailure.tokenExchangeFailed) would make both call sites say that on their own. Three lines, no behavior change — worth doing if you're touching the file for either of the other two comments.

[deaflynx]

Done in 272d62b: _asFailure(e, rejectedAs: NoAuthFailure.tokenExchangeFailed) / rejectedAs: NoAuthFailure.sessionInvalid.

[vvlladd28]

Re-review summary

Re-reviewed fix: make QR-code login and endpoint switch reliable (PROD-8200) — verified 3 finding(s) from previous review, covering the one commit pushed since (272d62b).

Status	Count
✅ Resolved	3
💬 Acknowledged	0
❌ Unresolved	0

All three fixed, each in the shape that was suggested rather than a heavier variant — cause is a DioExceptionType? and not the whole DioException, and it rides the existing catch log through toString() instead of adding a log call, so _asFailure stays side-effect free. The ?? '' guard on state.host was kept, correctly: the field is typed String?, so it's the type system asking, not defensiveness.

failedToConnectToServer is not orphaned by the switch — it still has two callers (tb_context.dart:258, tb_client_service.dart:117), where no host is in hand. Two keys that differ by whether a host is available is coherent, not duplication.

Verified by running the toolchain, since this repo has no CI

Last round I noted that nothing mechanically verifies this branch. Rather than leave that hanging, I resolved the thingsboard_ce_client path dependency and ran the real toolchain (Flutter 3.29.0, the version FVM pins here) against both this branch and its base.

Analyzer — no regression, and a small improvement:

	issues	errors
develop/1.9.0 (ae69084)	137	0
this branch (272d62b)	133	0

Diffing the two lists position-independently: zero new analyzer issues, and the PR removes four pre-existing warnings — dead_null_aware_expression in noauth_provider.dart plus three unused imports in home_handler.dart. So the description's "flutter analyze at the develop/1.9.0 baseline" understates it slightly; it is better than baseline.

Code generation — reproducible except for one file, which is the one new finding this round (below). Re-running intl_utils:generate produces the committed messages_en.dart and l10n.dart byte-for-byte, so the mNN renumbering this commit caused (the old m30 became m31, cascading through m34) is a genuine regen and not a hand-edit — the case worth checking, since a partial regen there would leave a dangling helper reference. Consistent with that, intl_en.arb parses with no duplicate keys (406 keys, 406 entries), and the new key follows the English-only convention the other four keys in this PR use, so the twelve other locales fall back through Intl.message's messageText with the host already interpolated. build_runner likewise reproduces every committed .freezed.dart and every .g.dart but one.

Additional findings

Both are about files outside this commit's diff.

• lib/core/auth/login/provider/login_provider.g.dart — this file is stale and is not part of the PR. login_provider.dart is changed by the PR (it's one of the central fixes — reading the live client through a getter instead of capturing it at construction), and _$loginHash() is generated from that source, so running build_runner on this branch rewrites it: 6aea9a4e… → 3d3570ba…. It's the only drift in the whole tree, and the only one of the four providers whose source this PR touches that didn't get its .g.dart regenerated — noauth_provider.g.dart, oauth_provider.g.dart and two_factor_confirm_provider.g.dart are all committed and all reproduce exactly. Low severity: the hash is debugGetCreateSourceHash, null in release, so nothing breaks at runtime and the analyzer is clean either way. Worth fixing anyway, because right now the next person to run build_runner gets an unrelated dirty file in their working tree, and it would fail a codegen-freshness check if one is ever added to CI. One command, one line of diff.
• pubspec.yaml — dio is imported directly by noauth_provider.dart but isn't a declared dependency, so it resolves transitively through thingsboard_ce_client (depend_on_referenced_packages). Pre-existing on develop/1.9.0, not introduced here, and tb_image_gallery_service.dart does the same — so nothing to fix in this PR. Worth mentioning only because this commit widens the exposure slightly: DioExceptionType is now the type of a field on SwitchEndpointFailure, not just a local import, so a dio major bump inside the client would surface here as a signature change rather than a local one. A one-line dio: entry would decouple it whenever that file is next touched.

Two small things inside the new commit are commented inline; both are nits and neither needs a respin on its own.

Finding details

• ✅ lib/core/auth/noauth/presentation/view/switch_endpoint_noauth_view.dart:187 — the connectionFailed arm was the only one not naming the host, and the arm where the host matters most — Fixed in code: new failedToConnectToHost key ("Failed to connect to {host}") in intl_en.arb, regenerated, and the arm uses it.
• ✅ lib/core/auth/noauth/provider/noauth_provider.dart:61 — the connectionFailed branch discarded the DioException, so a timeout, a refused connection and a bad certificate were indistinguishable in the logs — Fixed in code with the proportionate version: a DioExceptionType? cause field filled from e.type on both branches and printed by toString(), picked up by the existing catch log at switchEndpoint.
• ✅ lib/core/auth/noauth/provider/noauth_provider.dart:368 — _asFailure's second positional argument read as "classify as this" when it only applies on a 401 — Fixed in code: rejectedAs: at both call sites, and the doc comment updated to match.

---

This re-review was auto-generated. Findings may contain errors — please verify before applying changes.

[vvlladd28]

The doc now describes both diagnostics fields but is attached only to status, so an IDE shows this sentence when you hover status — including the part about cause — and shows nothing at all when you hover cause. Lifting the shared "diagnostics only, reaches the logs through toString" sentence to a plain // comment above the pair (or to the class doc) and giving each field its own one-liner would put the text where each reader will look for it.

[deaflynx]

Moved in 3cddf46: the shared diagnostics sentence is on the class doc of SwitchEndpointFailure, and status / cause each carry a one-line doc of their own.

[vvlladd28]

Small consistency nit now that a second arm needs the host: _stepMessage directly above hoists this once as final host = state.host ?? ''; and then uses host in every arm, while _errorMessage repeats state.host ?? '' in two. Mirroring the hoist here would keep the two switch bodies symmetrical and leave one place to change if a third host-bearing failure arm appears.

[deaflynx]

Hoisted in 3cddf46: _errorMessage now starts with final host = state.host ?? ''; like _stepMessage, and both host-bearing arms use it.

[deaflynx]

Re the two additional findings in review 5132287087, both taken in 3cddf46: login_provider.g.dart is regenerated (hash now 3d3570ba…, matching the predicted value), and dio: ^5.7.0 is declared directly in pubspec.yaml (same constraint as the client; the lock diff is the one-line transitive → direct change). The latter also clears the two pre-existing depend_on_referenced_packages hints, so the analyzer count is 130 against the 137 on develop/1.9.0.

[vvlladd28]

Re-review summary

Re-reviewed fix: make QR-code login and endpoint switch reliable (PROD-8200) — verified 4 finding(s) from previous review, covering the one commit pushed since (3cddf46).

Status	Count
✅ Resolved	4
💬 Acknowledged	0
❌ Unresolved	0

All four fixed, and no new findings — nothing inline this round. Two of the four were nits inside the last commit's diff; the other two were the codegen and dependency observations from the review body, which are claims about reproducibility rather than about code, so I confirmed them by running the toolchain instead of by reading.

This closes the loop on all six prior rounds. Every finding across the PR's 95 comments now has either a code change or an explicit, agreed deferral, and the three items under Still open in the description are correctly scoped out: the notify && !config.ignoreErrors fix and the storage-key export both belong in a thingsboard_client PR (the third depends on the first), and tests need the getIt collaborators injected through the notifier first — which is the shape both sides converged on.

Verified by running the toolchain, since this repo has no CI

Code generation now reproduces completely. Last round login_provider.g.dart was the one file in the tree that build_runner rewrote — the PR changes login_provider.dart, but its .g.dart had not been refreshed. After 3cddf46, dart run build_runner build --delete-conflicting-outputs writes 37 outputs and leaves git status --porcelain completely empty. The committed hash is also exactly the one last round predicted (6aea9a4e… → 3d3570ba…), which is the expected result rather than a coincidence: login_provider.dart was last touched in the PR's first commit (16c9bd9), so the source the hash derives from has not changed since that measurement.

The dio declaration does what it was meant to do. flutter analyze reports 0 errors, and no depend_on_referenced_packages warning mentioning dio remains — the seven left are for other packages and are all pre-existing on develop/1.9.0. The declared ^5.7.0 is also exactly the constraint thingsboard_ce_client itself declares, so the two cannot drift apart, and pubspec.lock still resolves the same 5.8.0+1: the only change in the lock is transitive → direct main, with no version movement anywhere in the file.

On the analyzer count specifically — I am deliberately not reporting a number against the description's. thingsboard_ce_client is a path dependency with no version pin, and about two dozen of the reported issues (unnecessary_non_null_assertion, invalid_null_aware_operator, undefined_hidden_name, dead_null_aware_expression) come from the nullability of the client's generated API surface. The total therefore moves with whatever the neighbouring client checkout happens to be, so my count and yours are not comparable and a one-off difference means nothing. 0 errors and "no new issues attributable to this PR" are the parts that survive across checkouts, and both hold.

Server-side assumptions, re-confirmed against thingsboard-pe

The classification logic rests on a few statements about server behaviour that are asserted in code comments. Since this is the last round, I checked each against the server source rather than leaving them as received wisdom — all four hold:

• /api/noauth/qr/{secret} really is permitAll (ThingsboardSecurityConfiguration: /api/noauth/** is in NON_TOKEN_BASED_AUTH_ENTRY_POINTS, .permitAll()), so _exchangeSecret's comment is right that a 401 there can only be a rejected secret.
• JWT_TOKEN_EXPIRED maps to 401 (ThingsboardErrorResponseHandler:111), so _verifySession mapping 401 → sessionInvalid is exact, not heuristic. The response body's "Token has expired" is also hardcoded English at :321, which is what justifies the view preferring localized copy over serverMessage.
• host= is always scheme-qualified. Worth stating precisely, because it is the assumption _isSameOrigin and _resolveHost depend on and it is not only MiscUtils.constructBaseUrl ("%s://%s:%d"). The value can also come from an admin-entered baseUrl in general settings, which is free text — but DefaultSystemSecurityService.formatBaseUrl prepends https:// whenever the string does not already start with http:// or https://. So Uri.parse(host).origin cannot throw on a server-supplied host=, and the "unusable target ends on the error screen" branch is genuinely reserved for hand-made or corrupted links.
• A 200 with no token cannot come from a TB server. MobileAppSecretServiceImpl.getJwtPair either returns a cached tokenFactory.createTokenPair(...) — where the access token is always present — or throws JWT_TOKEN_EXPIRED, i.e. a 401. The token == null guard in _exchangeSecret is therefore defensive against something in front of the server (a captive portal or proxy answering 200 with HTML), which is a real scenario for a QR pointing at an arbitrary self-hosted host, so the guard is worth keeping — it just is not a TB response shape.

Finding details

• ✅ lib/core/auth/noauth/provider/noauth_provider.dart:47 — the shared diagnostics doc described both status and cause but was attached only to status, so hovering cause showed nothing — Fixed in code: the shared sentence moved to the class doc of SwitchEndpointFailure, and status and cause each carry a one-liner of their own. Verified in the file that both /// blocks attach to the field that follows them, which is the part that was actually broken.
• ✅ lib/core/auth/noauth/presentation/view/switch_endpoint_noauth_view.dart:180 — _errorMessage repeated state.host ?? '' in two arms while _stepMessage directly above hoisted it once — Fixed in code: _errorMessage now opens with final host = state.host ?? ''; and both host-bearing arms use it, mirroring _stepMessage.
• ✅ lib/core/auth/login/provider/login_provider.g.dart (review body) — the file was stale relative to its regenerated source — Fixed in code, and confirmed by running build_runner: the tree comes back clean.
• ✅ pubspec.yaml (review body) — dio was imported directly but resolved transitively — Fixed in code: dio: ^5.7.0, matching the client's own constraint. Confirmed that the dio lint is gone and that the lock records no version movement.

---

This re-review was auto-generated. Findings may contain errors — please verify before applying changes.