Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 68 additions & 0 deletions openspec/changes/graceful-401-session-dialog/design.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
## Context

The app uses a global `AuthInterceptor` on Dio that catches 401 responses and immediately calls `onUnauthorized()` → `auth.logout()`. The error still propagates back to callers, causing visible error banners before the widget tree is rebuilt.

`ApiException.fromDioException()` already extracts the human-readable message via `extractApiMessage()`. So `ApiException.message` for a 401 is already clean (e.g. `"Please Login and try again."`).

`_AppShellBuilder` in `app_router.dart` already hosts a global bottom sheet overlay for logout confirmation — the same pattern applies for the session dialog.

## Goals / Non-Goals

**Goals:**
- On any 401: show one blocking dialog app-wide with the backend's message.
- User must explicitly tap "Sign In Again" to trigger logout.
- No error banners from sync providers on 401.
- Dialog fires only once even if multiple 401s arrive simultaneously.

**Non-Goals:**
- Changing the logout state machine or GoRouter redirect logic.
- Token refresh / silent re-auth.
- Showing the dialog on 401s from auth-flow endpoints (login, OTP, etc.).

## Decisions

### Decision: `sessionExpiredProvider` holds `String?` not `bool`

Holding the message string (not just a flag) lets the dialog render the backend message directly. `null` = no dialog shown. Non-null string = dialog is visible with that message.

**Alternative**: Separate `bool` flag + `String` message as two providers.
**Rejected**: Unnecessary complexity; a nullable string is a clean sentinel.

### Decision: Callback renamed from `onUnauthorized` to `onSessionExpired(String message)`

The new callback signature passes the extracted message. `AuthInterceptor` extracts the message from `DioException` (or falls back to a default) before calling the callback.

**Alternative**: Keep `onUnauthorized()` with no args, read message from somewhere else.
**Rejected**: Coupling concern — the interceptor already has the message; passing it is cleaner.

### Decision: Suppress 401 errors in sync providers before they reach UI state

In `course_list_provider._performSync()` and `_performSearch()`, if the caught exception is `ApiErrorType.unauthorized`, skip writing to `courseListSyncError` / `state.error`. The dialog handles UX globally.

Detection: `e is ApiException && e.type == ApiErrorType.unauthorized`, with fallback for raw `DioException` with `statusCode == 401`.

### Decision: Dialog is rendered as a Stack overlay in `_AppShellBuilder`

Same pattern as the logout bottom sheet. The `sessionExpiredProvider` is watched inside `_AppShellBuilder`; when non-null, a full-screen semi-transparent overlay + centered dialog card is shown above all content.

**Alternative**: Use `showDialog()` imperatively via a navigator listener.
**Rejected**: Declarative is safer — avoids context timing issues with GoRouter and avoids needing a BuildContext at interceptor time.

### Decision: `SessionExpiredDialog` lives in `packages/core/lib/widgets/`

It depends only on core design tokens and is reusable across any package.

## Risks / Trade-offs

- **Risk**: Multiple 401s fire before `sessionExpiredProvider` is set → multiple `setState` calls. The `_isLoggingOut` guard in `AuthInterceptor` already prevents multiple `onSessionExpired` calls. The provider itself is idempotent (same message written twice is fine).
- **Risk**: 401 from an auth-flow endpoint triggers the dialog. → Mitigated by the existing `_authFlowPaths` skip list in `AuthInterceptor`.

## Migration Plan

1. Add `sessionExpiredProvider` to `core`.
2. Update `AuthInterceptor` callback signature.
3. Update `dio_provider.dart` wiring.
4. Add `SessionExpiredDialog` widget in `core`.
5. Update `_AppShellBuilder` to watch provider and render overlay.
6. Suppress 401 errors in `course_list_provider.dart`.
7. Delete the old `suppress-401-error-display` change (it is superseded by this one).
33 changes: 33 additions & 0 deletions openspec/changes/graceful-401-session-dialog/proposal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
## Why

When the backend invalidates a session token, any in-flight API call returns a 401 with a human-readable message (e.g., `"Please Login and try again."`). Today the app:
1. Flashes the raw JSON error object in a red banner on screen
2. Immediately auto-logs out the user with no explanation

This is confusing and feels like a crash. The fix is to intercept 401s globally, suppress all error UI, and show a single non-dismissible dialog that displays the backend message and gives the user one clear action: sign in again.

## What Changes

- `AuthInterceptor.onUnauthorized` callback is replaced with `onSessionExpired(String message)` — passes the extracted API message instead of calling logout directly.
- A new `sessionExpiredProvider` (`StateProvider<String?>`) is added in `core` to hold the expiry message. `null` = no dialog. Non-null = show dialog with that message.
- `dio_provider.dart` is updated to wire the new callback: set `sessionExpiredProvider.state = message` on 401.
- `_AppShellBuilder` in `app_router.dart` watches `sessionExpiredProvider` and renders a non-dismissible `SessionExpiredDialog` overlay when non-null.
- The dialog displays the API message and a "Sign In Again" button. Tapping calls `auth.logout()`.
- All sync providers (`course_list_provider.dart`) suppress 401 errors — they must NOT write to visible error state when a 401 fires (since the dialog handles it globally).

## Capabilities

### New Capabilities
- `session-expired-dialog`: A global, non-dismissible dialog overlay that intercepts 401 Unauthorized responses, displays the backend-provided message, and gives the user a single "Sign In Again" action to trigger logout.

### Modified Capabilities
- None — the logout flow and auth state machine are unchanged.

## Impact

- `packages/core/lib/network/auth_interceptor.dart` — callback signature change
- `packages/core/lib/network/dio_provider.dart` — wiring the new callback
- `packages/core/lib/data/auth/` — new `sessionExpiredProvider`
- `packages/testpress/lib/navigation/app_router.dart` — dialog overlay in `_AppShellBuilder`
- `packages/courses/lib/providers/course_list_provider.dart` — suppress 401 errors from surfacing in state
- A new `SessionExpiredDialog` widget (in `core` or `testpress`)
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
## ADDED Requirements

### Requirement: sessionExpiredProvider holds the 401 message
The system SHALL expose a `StateProvider<String?>` named `sessionExpiredProvider` in `core`. A `null` value means no session expiry is in progress. A non-null value is the human-readable message extracted from the 401 API response.

#### Scenario: No active session expiry
- **WHEN** no 401 has been received
- **THEN** `sessionExpiredProvider.state` is `null`

#### Scenario: 401 received from a protected endpoint
- **WHEN** `AuthInterceptor.onError` processes a 401 from a non-auth-flow path
- **THEN** `sessionExpiredProvider.state` is set to the message from `ApiException.message`
- **THEN** the value is non-null and non-empty

#### Scenario: Fallback message when backend sends no message
- **WHEN** the 401 response body contains no extractable message
- **THEN** `sessionExpiredProvider.state` is set to a default string: `"Your session has expired. Please sign in again."`

### Requirement: AuthInterceptor uses onSessionExpired callback with message
The `AuthInterceptor` SHALL replace `onUnauthorized: void Function()` with `onSessionExpired: void Function(String message)`. On a 401 from a non-auth-flow path, it SHALL extract the message from the `DioException` and call `onSessionExpired(message)` exactly once per session.

#### Scenario: First 401 triggers callback
- **WHEN** a 401 DioException arrives on a protected endpoint
- **THEN** `onSessionExpired` is called once with the extracted message
- **THEN** `_isLoggingOut` is set to `true` to prevent repeated calls

#### Scenario: Subsequent 401s are ignored
- **WHEN** a second 401 arrives while `_isLoggingOut == true`
- **THEN** `onSessionExpired` is NOT called again
- **THEN** `sessionExpiredProvider.state` is unchanged

#### Scenario: Auth-flow endpoints are excluded
- **WHEN** a 401 arrives on a path in `_authFlowPaths` (login, OTP, etc.)
- **THEN** `onSessionExpired` is NOT called
- **THEN** the error propagates normally to the caller

### Requirement: SessionExpiredDialog is a non-dismissible blocking overlay
The system SHALL render a `SessionExpiredDialog` widget when `sessionExpiredProvider` is non-null. The dialog MUST NOT be dismissible by tapping outside or pressing back.

#### Scenario: Dialog appears on session expiry
- **WHEN** `sessionExpiredProvider.state` becomes non-null
- **THEN** a full-screen semi-transparent overlay is shown above all app content
- **THEN** a centered card dialog is shown with the message from `sessionExpiredProvider.state`
- **THEN** a single "Sign In Again" button is visible

#### Scenario: Dialog cannot be dismissed without action
- **WHEN** the user taps outside the dialog card
- **THEN** the dialog remains visible
- **WHEN** the user presses the device back button
- **THEN** the dialog remains visible

#### Scenario: User taps "Sign In Again"
- **WHEN** the user taps the "Sign In Again" button
- **THEN** `auth.logout()` is called
- **THEN** `sessionExpiredProvider.state` is reset to `null`
- **THEN** the app navigates to the login screen (GoRouter redirect handles this automatically)

### Requirement: Sync providers MUST NOT surface 401 errors in visible UI state
When a 401 is caught in any background sync provider, it SHALL be silently discarded and MUST NOT be written to any provider state that drives visible error UI.

#### Scenario: Study screen sync catches a 401
- **WHEN** `CourseList._performSync()` or `CourseSearch._performSearch()` catches an `ApiException` with `type == ApiErrorType.unauthorized`
- **THEN** `courseListSyncError` is NOT updated
- **THEN** `CourseSearchState.error` is NOT set
- **THEN** no red error banner appears in `StudyScreen`

#### Scenario: Non-401 sync errors are still shown
- **WHEN** a sync provider catches an error that is NOT `ApiErrorType.unauthorized`
- **THEN** the error is written to state as before (existing behaviour preserved)
34 changes: 34 additions & 0 deletions openspec/changes/graceful-401-session-dialog/tasks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
## 1. Core: Session Expired Provider

- [x] 1.1 Add `sessionExpiredProvider` as `StateProvider<String?>` in `packages/core/lib/data/auth/auth_provider.dart` (initial state: `null`)

## 2. Core: AuthInterceptor Callback Update

- [x] 2.1 Rename `onUnauthorized: void Function()?` to `onSessionExpired: void Function(String message)?` in `AuthInterceptor`
- [x] 2.2 In `AuthInterceptor.onError()`, extract the message from the `DioException` using `ApiException.fromDioException(err).message` before calling `onSessionExpired`
- [x] 2.3 Apply a fallback message `"Your session has expired. Please sign in again."` when the extracted message is empty

## 3. Core: Dio Provider Wiring

- [x] 3.1 Update `dio_provider.dart` — replace `onUnauthorized: () => ref.read(authProvider.notifier).logout()` with `onSessionExpired: (msg) => ref.read(sessionExpiredProvider.notifier).state = msg`

## 4. Core: SessionExpiredDialog Widget

- [x] 4.1 Create `packages/core/lib/widgets/session_expired_dialog.dart` — a `ConsumerWidget` that accepts `message` and `onSignIn` callback
- [x] 4.2 Style: semi-transparent full-screen barrier + centered card with lock icon, title "Session Ended", message text, and "Sign In Again" primary button
- [x] 4.3 Make it non-dismissible: `WillPopScope` (or `PopScope`) returning false, no barrier tap dismissal

## 5. App Shell: Dialog Overlay

- [x] 5.1 In `_AppShellBuilder` (`app_router.dart`), watch `sessionExpiredProvider`
- [x] 5.2 When `sessionExpiredProvider` is non-null, render `SessionExpiredDialog` as a `Stack` overlay above the `AppShell` (same pattern as the logout bottom sheet)
- [x] 5.3 Wire the `onSignIn` callback: call `auth.logout()` then reset `sessionExpiredProvider.state = null`

## 6. Courses: Suppress 401 Errors from UI State

- [x] 6.1 In `course_list_provider._performSync()` catch block — if `e is ApiException && e.type == ApiErrorType.unauthorized`, return early without setting `courseListSyncError`
- [x] 6.2 In `CourseSearch._performSearch()` catch block — same guard, skip `state.copyWith(error: e)` on unauthorized

## 7. Cleanup

- [x] 7.1 Export `sessionExpiredProvider` and `SessionExpiredDialog` from `core.dart`
1 change: 1 addition & 0 deletions packages/core/lib/core.dart
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export 'widgets/dashboard_header.dart';
export 'widgets/lesson_detail_shell.dart';
export 'widgets/bookmark_folders_sheet.dart';
export 'widgets/app_toast.dart';
export 'widgets/session_expired_dialog.dart';

// Shell
export 'shell/app_shell.dart';
Expand Down
11 changes: 11 additions & 0 deletions packages/core/lib/data/auth/auth_provider.dart
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import 'package:riverpod_annotation/riverpod_annotation.dart';
import 'package:flutter_riverpod/flutter_riverpod.dart';
import 'package:google_sign_in/google_sign_in.dart';

import '../config/app_config.dart';
Expand Down Expand Up @@ -42,6 +43,16 @@ final authRepositoryProvider = Provider<AuthRepository>((ref) {
);
});

/// Holds the session-expired message from a 401 response.
/// `null` = no session expiry in progress.
/// Non-null = show the SessionExpiredDialog with this message.
final sessionExpiredProvider = StateProvider<String?>((ref) => null);

/// Tracks whether the onboarding screen has already been shown this app session.
/// Stored in Riverpod so it resets between widget tests and stays consistent
/// with how the rest of UI state is managed.
final hasShownOnboardingProvider = StateProvider<bool>((ref) => false);

@Riverpod(keepAlive: true)
class Auth extends _$Auth {
AuthRepository get _repository => ref.read(authRepositoryProvider);
Expand Down
21 changes: 19 additions & 2 deletions packages/core/lib/data/exceptions/api_exception.dart
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import 'dart:convert';
import 'package:dio/dio.dart';

enum ApiErrorType {
Expand Down Expand Up @@ -84,7 +85,10 @@ class ApiException implements Exception {

if (statusCode == 401) {
return ApiException(
backendMessage ?? 'You are not authorized to perform this action.',
// Pass the backend message as-is, or empty string if none.
// An empty message signals SessionExpiredDialog to show the
// localized sessionExpiredFallbackMessage in the user's locale.
backendMessage ?? '',
type: ApiErrorType.unauthorized,
statusCode: statusCode,
data: data,
Expand Down Expand Up @@ -152,7 +156,20 @@ class ApiException implements Exception {
static String? extractApiMessage(dynamic responseData) {
if (responseData == null) return null;
if (responseData is String && responseData.trim().isNotEmpty) {
return responseData.trim();
final trimmed = responseData.trim();
if ((trimmed.startsWith('{') && trimmed.endsWith('}')) ||
(trimmed.startsWith('[') && trimmed.endsWith(']'))) {
try {
final decoded = jsonDecode(trimmed);
final nestedMsg = extractApiMessage(decoded);
if (nestedMsg != null && nestedMsg.isNotEmpty) {
return nestedMsg;
}
} catch (_) {
// Fall through to returning raw string if JSON parsing fails
}
}
return trimmed;
}
if (responseData is List) {
final messages = <String>[];
Expand Down
24 changes: 24 additions & 0 deletions packages/core/lib/generated/l10n/app_localizations.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5038,6 +5038,30 @@ abstract class AppLocalizations {
/// In en, this message translates to:
/// **'View'**
String get viewAction;

/// No description provided for @sessionExpiredTitle.
///
/// In en, this message translates to:
/// **'Session Ended'**
String get sessionExpiredTitle;

/// No description provided for @sessionExpiredIconSemantics.
///
/// In en, this message translates to:
/// **'Session ended icon'**
String get sessionExpiredIconSemantics;

/// No description provided for @sessionExpiredLoginButton.
///
/// In en, this message translates to:
/// **'Login Again'**
String get sessionExpiredLoginButton;

/// No description provided for @sessionExpiredFallbackMessage.
///
/// In en, this message translates to:
/// **'Your session has expired. Please sign in again.'**
String get sessionExpiredFallbackMessage;
}

class _AppLocalizationsDelegate
Expand Down
13 changes: 13 additions & 0 deletions packages/core/lib/generated/l10n/app_localizations_ar.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2792,4 +2792,17 @@ class AppLocalizationsAr extends AppLocalizations {

@override
String get viewAction => 'عرض';

@override
String get sessionExpiredTitle => 'انتهت الجلسة';

@override
String get sessionExpiredIconSemantics => 'أيقونة انتهاء الجلسة';

@override
String get sessionExpiredLoginButton => 'تسجيل الدخول مرة أخرى';

@override
String get sessionExpiredFallbackMessage =>
'انتهت صلاحية جلستك. يرجى تسجيل الدخول مرة أخرى.';
}
13 changes: 13 additions & 0 deletions packages/core/lib/generated/l10n/app_localizations_en.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2790,4 +2790,17 @@ class AppLocalizationsEn extends AppLocalizations {

@override
String get viewAction => 'View';

@override
String get sessionExpiredTitle => 'Session Ended';

@override
String get sessionExpiredIconSemantics => 'Session ended icon';

@override
String get sessionExpiredLoginButton => 'Login Again';

@override
String get sessionExpiredFallbackMessage =>
'Your session has expired. Please sign in again.';
}
13 changes: 13 additions & 0 deletions packages/core/lib/generated/l10n/app_localizations_ml.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2842,4 +2842,17 @@ class AppLocalizationsMl extends AppLocalizations {

@override
String get viewAction => 'കാണുക';

@override
String get sessionExpiredTitle => 'സെഷൻ അവസാനിച്ചു';

@override
String get sessionExpiredIconSemantics => 'സെഷൻ അവസാനിച്ചതിന്റെ ഐക്കൺ';

@override
String get sessionExpiredLoginButton => 'വീണ്ടും ലോഗിൻ ചെയ്യുക';

@override
String get sessionExpiredFallbackMessage =>
'നിങ്ങളുടെ സെഷൻ കാലഹരണപ്പെട്ടു. ദയവായി വീണ്ടും ലോഗിൻ ചെയ്യുക.';
}
13 changes: 13 additions & 0 deletions packages/core/lib/generated/l10n/app_localizations_ta.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2835,4 +2835,17 @@ class AppLocalizationsTa extends AppLocalizations {

@override
String get viewAction => 'காண்க';

@override
String get sessionExpiredTitle => 'அமர்வு முடிந்தது';

@override
String get sessionExpiredIconSemantics => 'அமர்வு முடிந்ததற்கான ஐகான்';

@override
String get sessionExpiredLoginButton => 'மீண்டும் உள்நுழைக';

@override
String get sessionExpiredFallbackMessage =>
'உங்கள் அமர்வு காலாவதியாகிவிட்டது. தயவுசெய்து மீண்டும் உள்நுழையவும்.';
}
7 changes: 6 additions & 1 deletion packages/core/lib/l10n/app_ar.arb
Original file line number Diff line number Diff line change
Expand Up @@ -1074,4 +1074,9 @@
"downloadCompleted": "اكتمل التنزيل",
"downloadStarted": "بدأ التنزيل",
"downloadingFile": "جاري التنزيل...",
"viewAction": "عرض"}
"viewAction": "عرض",
"sessionExpiredTitle": "انتهت الجلسة",
"sessionExpiredIconSemantics": "أيقونة انتهاء الجلسة",
"sessionExpiredLoginButton": "تسجيل الدخول مرة أخرى",
"sessionExpiredFallbackMessage": "انتهت صلاحية جلستك. يرجى تسجيل الدخول مرة أخرى."
}
Loading