feat(logging): add logging to auth feature
This commit is contained in:
@@ -111,3 +111,91 @@ This file governs `apps/**` (Flutter). Keep rules strict, short, and reusable.
|
|||||||
- Prioritize tests for model parsing, service logic, and high-regression interaction flows.
|
- Prioritize tests for model parsing, service logic, and high-regression interaction flows.
|
||||||
- Simple static UI changes may skip tests.
|
- Simple static UI changes may skip tests.
|
||||||
- Auth/Home/Cache changes must include targeted regression tests.
|
- Auth/Home/Cache changes must include targeted regression tests.
|
||||||
|
|
||||||
|
## Logging Conventions (Must)
|
||||||
|
|
||||||
|
### Logger Setup
|
||||||
|
|
||||||
|
```dart
|
||||||
|
import 'core/logging/logger.dart';
|
||||||
|
|
||||||
|
class SomeBloc extends Cubit<SomeState> {
|
||||||
|
final Logger _logger = getLogger('features.<feature>.<component>');
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
### Log Level Policy
|
||||||
|
|
||||||
|
| Level | When to Use | Noise Level |
|
||||||
|
|-------|-------------|-------------|
|
||||||
|
| **error** | All exceptions and failures - MUST log every error site | Required, never skip |
|
||||||
|
| **warning** | Degraded behavior, retry, fallback, malformed data | Minimal, only when action taken |
|
||||||
|
| **info** | Key business events (login, logout, send message) | Minimal, only milestone events |
|
||||||
|
| **debug** | Detailed flow tracing (only in debug builds) | High, avoid in release |
|
||||||
|
|
||||||
|
### Error Logging Requirements
|
||||||
|
|
||||||
|
**Every try-catch that handles an exception MUST log it:**
|
||||||
|
```dart
|
||||||
|
try {
|
||||||
|
await _repository.someOperation();
|
||||||
|
} catch (e, stackTrace) {
|
||||||
|
_logger.error(
|
||||||
|
message: 'Operation failed: $operationName',
|
||||||
|
error: e,
|
||||||
|
stackTrace: stackTrace,
|
||||||
|
extra: {'context': 'relevant_data'},
|
||||||
|
);
|
||||||
|
// handle error
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
### Info Logging Requirements
|
||||||
|
|
||||||
|
**Only log these milestone events:**
|
||||||
|
- User login/logout
|
||||||
|
- Message sent/received
|
||||||
|
- Data sync completed
|
||||||
|
- Important state transitions
|
||||||
|
|
||||||
|
```dart
|
||||||
|
_logger.info(
|
||||||
|
message: 'User logged in',
|
||||||
|
extra: {'user_id': user.id},
|
||||||
|
);
|
||||||
|
```
|
||||||
|
|
||||||
|
### Warning Logging Requirements
|
||||||
|
|
||||||
|
**Only log when taking corrective action:**
|
||||||
|
- Retrying after failure
|
||||||
|
- Using fallback data
|
||||||
|
- Skipping malformed data
|
||||||
|
- Deprecation warnings
|
||||||
|
|
||||||
|
```dart
|
||||||
|
_logger.warning(
|
||||||
|
message: 'Cache miss, loading from remote',
|
||||||
|
extra: {'key': cacheKey},
|
||||||
|
);
|
||||||
|
```
|
||||||
|
|
||||||
|
### Module Naming Convention
|
||||||
|
|
||||||
|
| Feature | Module Path |
|
||||||
|
|---------|------------|
|
||||||
|
| auth | `features.auth` |
|
||||||
|
| calendar | `features.calendar` |
|
||||||
|
| chat | `features.chat` |
|
||||||
|
| contacts | `features.contacts` |
|
||||||
|
| home | `features.home` |
|
||||||
|
| messages | `features.messages` |
|
||||||
|
| settings | `features.settings` |
|
||||||
|
| todo | `features.todo` |
|
||||||
|
|
||||||
|
### Prohibited Practices
|
||||||
|
|
||||||
|
- **Never** log sensitive data: passwords, tokens, PII, message content
|
||||||
|
- **Never** log at debug level in production (release mode)
|
||||||
|
- **Never** skip error logging even if you "handle" the error
|
||||||
|
- **Never** log for every iteration in loops - only on failures
|
||||||
|
|||||||
@@ -96,7 +96,7 @@ class LogService {
|
|||||||
void info({
|
void info({
|
||||||
required String message,
|
required String message,
|
||||||
required String module,
|
required String module,
|
||||||
required Map<String, dynamic> extra,
|
Map<String, dynamic>? extra,
|
||||||
StackTrace? stackTrace,
|
StackTrace? stackTrace,
|
||||||
}) {
|
}) {
|
||||||
final trace = stackTrace ?? StackTrace.current;
|
final trace = stackTrace ?? StackTrace.current;
|
||||||
@@ -118,7 +118,7 @@ class LogService {
|
|||||||
void warning({
|
void warning({
|
||||||
required String message,
|
required String message,
|
||||||
required String module,
|
required String module,
|
||||||
required Map<String, dynamic> extra,
|
Map<String, dynamic>? extra,
|
||||||
StackTrace? stackTrace,
|
StackTrace? stackTrace,
|
||||||
}) {
|
}) {
|
||||||
final trace = stackTrace ?? StackTrace.current;
|
final trace = stackTrace ?? StackTrace.current;
|
||||||
|
|||||||
@@ -33,23 +33,23 @@ class Logger {
|
|||||||
|
|
||||||
void info({
|
void info({
|
||||||
required String message,
|
required String message,
|
||||||
required Map<String, dynamic> extra,
|
Map<String, dynamic>? extra,
|
||||||
StackTrace? stackTrace,
|
StackTrace? stackTrace,
|
||||||
}) => _service.info(
|
}) => _service.info(
|
||||||
message: message,
|
message: message,
|
||||||
module: module,
|
module: module,
|
||||||
extra: extra,
|
extra: extra ?? {},
|
||||||
stackTrace: stackTrace,
|
stackTrace: stackTrace,
|
||||||
);
|
);
|
||||||
|
|
||||||
void warning({
|
void warning({
|
||||||
required String message,
|
required String message,
|
||||||
required Map<String, dynamic> extra,
|
Map<String, dynamic>? extra,
|
||||||
StackTrace? stackTrace,
|
StackTrace? stackTrace,
|
||||||
}) => _service.warning(
|
}) => _service.warning(
|
||||||
message: message,
|
message: message,
|
||||||
module: module,
|
module: module,
|
||||||
extra: extra,
|
extra: extra ?? {},
|
||||||
stackTrace: stackTrace,
|
stackTrace: stackTrace,
|
||||||
);
|
);
|
||||||
|
|
||||||
|
|||||||
@@ -1,10 +1,12 @@
|
|||||||
import 'package:flutter_bloc/flutter_bloc.dart';
|
import 'package:flutter_bloc/flutter_bloc.dart';
|
||||||
|
import '../../../../core/logging/logger.dart';
|
||||||
import '../../data/repositories/auth_repository.dart';
|
import '../../data/repositories/auth_repository.dart';
|
||||||
import 'auth_event.dart';
|
import 'auth_event.dart';
|
||||||
import 'auth_state.dart';
|
import 'auth_state.dart';
|
||||||
|
|
||||||
class AuthBloc extends Bloc<AuthEvent, AuthState> {
|
class AuthBloc extends Bloc<AuthEvent, AuthState> {
|
||||||
final AuthRepository _repository;
|
final AuthRepository _repository;
|
||||||
|
final Logger _logger = getLogger('features.auth.bloc');
|
||||||
|
|
||||||
AuthBloc(this._repository) : super(AuthInitial()) {
|
AuthBloc(this._repository) : super(AuthInitial()) {
|
||||||
on<AuthStarted>(_onStarted);
|
on<AuthStarted>(_onStarted);
|
||||||
@@ -19,6 +21,10 @@ class AuthBloc extends Bloc<AuthEvent, AuthState> {
|
|||||||
final refreshToken = await _repository.getRefreshToken();
|
final refreshToken = await _repository.getRefreshToken();
|
||||||
if (refreshToken != null) {
|
if (refreshToken != null) {
|
||||||
final response = await _repository.refreshSession(refreshToken);
|
final response = await _repository.refreshSession(refreshToken);
|
||||||
|
_logger.info(
|
||||||
|
message: 'Session refreshed successfully',
|
||||||
|
extra: {'user_id': response.user.id},
|
||||||
|
);
|
||||||
emit(
|
emit(
|
||||||
AuthAuthenticated(
|
AuthAuthenticated(
|
||||||
user: AuthUser(id: response.user.id, phone: response.user.phone),
|
user: AuthUser(id: response.user.id, phone: response.user.phone),
|
||||||
@@ -29,11 +35,20 @@ class AuthBloc extends Bloc<AuthEvent, AuthState> {
|
|||||||
emit(
|
emit(
|
||||||
const AuthUnauthenticated(reason: AuthUnauthenticatedReason.signedOut),
|
const AuthUnauthenticated(reason: AuthUnauthenticatedReason.signedOut),
|
||||||
);
|
);
|
||||||
} catch (_) {
|
} catch (e, stackTrace) {
|
||||||
|
_logger.error(
|
||||||
|
message: 'Session refresh failed',
|
||||||
|
error: e,
|
||||||
|
stackTrace: stackTrace,
|
||||||
|
);
|
||||||
try {
|
try {
|
||||||
await _repository.clearSessionLocalOnly();
|
await _repository.clearSessionLocalOnly();
|
||||||
} catch (_) {
|
} catch (e, stackTrace) {
|
||||||
// Keep state convergence even when storage cleanup fails.
|
_logger.error(
|
||||||
|
message: 'Failed to clear local session',
|
||||||
|
error: e,
|
||||||
|
stackTrace: stackTrace,
|
||||||
|
);
|
||||||
} finally {
|
} finally {
|
||||||
emit(
|
emit(
|
||||||
const AuthUnauthenticated(
|
const AuthUnauthenticated(
|
||||||
@@ -45,6 +60,7 @@ class AuthBloc extends Bloc<AuthEvent, AuthState> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
void _onLoggedIn(AuthLoggedIn event, Emitter<AuthState> emit) {
|
void _onLoggedIn(AuthLoggedIn event, Emitter<AuthState> emit) {
|
||||||
|
_logger.info(message: 'User logged in', extra: {'user_id': event.user.id});
|
||||||
emit(AuthAuthenticated(user: event.user));
|
emit(AuthAuthenticated(user: event.user));
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -54,8 +70,13 @@ class AuthBloc extends Bloc<AuthEvent, AuthState> {
|
|||||||
) async {
|
) async {
|
||||||
try {
|
try {
|
||||||
await _repository.deleteSession();
|
await _repository.deleteSession();
|
||||||
} catch (_) {
|
_logger.info(message: 'User logged out');
|
||||||
// Keep state convergence even when logout cleanup fails.
|
} catch (e, stackTrace) {
|
||||||
|
_logger.error(
|
||||||
|
message: 'Failed to delete session on logout',
|
||||||
|
error: e,
|
||||||
|
stackTrace: stackTrace,
|
||||||
|
);
|
||||||
} finally {
|
} finally {
|
||||||
emit(
|
emit(
|
||||||
const AuthUnauthenticated(reason: AuthUnauthenticatedReason.signedOut),
|
const AuthUnauthenticated(reason: AuthUnauthenticatedReason.signedOut),
|
||||||
@@ -67,10 +88,15 @@ class AuthBloc extends Bloc<AuthEvent, AuthState> {
|
|||||||
AuthSessionInvalidated event,
|
AuthSessionInvalidated event,
|
||||||
Emitter<AuthState> emit,
|
Emitter<AuthState> emit,
|
||||||
) async {
|
) async {
|
||||||
|
_logger.warning(message: 'Session invalidated by server');
|
||||||
try {
|
try {
|
||||||
await _repository.clearSessionLocalOnly();
|
await _repository.clearSessionLocalOnly();
|
||||||
} catch (_) {
|
} catch (e, stackTrace) {
|
||||||
// Keep state convergence even when local cleanup fails.
|
_logger.error(
|
||||||
|
message: 'Failed to clear local session',
|
||||||
|
error: e,
|
||||||
|
stackTrace: stackTrace,
|
||||||
|
);
|
||||||
} finally {
|
} finally {
|
||||||
emit(
|
emit(
|
||||||
const AuthUnauthenticated(reason: AuthUnauthenticatedReason.expired),
|
const AuthUnauthenticated(reason: AuthUnauthenticatedReason.expired),
|
||||||
|
|||||||
@@ -5,6 +5,7 @@ import 'package:formz/formz.dart';
|
|||||||
import 'package:equatable/equatable.dart';
|
import 'package:equatable/equatable.dart';
|
||||||
import '../../../../data/network/api_exception.dart';
|
import '../../../../data/network/api_exception.dart';
|
||||||
import '../../../../core/l10n/l10n.dart';
|
import '../../../../core/l10n/l10n.dart';
|
||||||
|
import '../../../../core/logging/logger.dart';
|
||||||
import '../../data/repositories/auth_repository.dart';
|
import '../../data/repositories/auth_repository.dart';
|
||||||
import '../../data/models/auth_response.dart';
|
import '../../data/models/auth_response.dart';
|
||||||
import '../../../../shared/forms/inputs.dart';
|
import '../../../../shared/forms/inputs.dart';
|
||||||
@@ -78,6 +79,7 @@ class LoginState extends Equatable {
|
|||||||
|
|
||||||
class LoginCubit extends Cubit<LoginState> {
|
class LoginCubit extends Cubit<LoginState> {
|
||||||
final AuthRepository _repository;
|
final AuthRepository _repository;
|
||||||
|
final Logger _logger = getLogger('features.auth.login');
|
||||||
Timer? _resendTimer;
|
Timer? _resendTimer;
|
||||||
|
|
||||||
LoginCubit(this._repository) : super(const LoginState());
|
LoginCubit(this._repository) : super(const LoginState());
|
||||||
@@ -149,10 +151,16 @@ class LoginCubit extends Cubit<LoginState> {
|
|||||||
),
|
),
|
||||||
);
|
);
|
||||||
return true;
|
return true;
|
||||||
} catch (e) {
|
} catch (e, stackTrace) {
|
||||||
if (isClosed) {
|
if (isClosed) {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
_logger.error(
|
||||||
|
message: 'Failed to send OTP',
|
||||||
|
error: e,
|
||||||
|
stackTrace: stackTrace,
|
||||||
|
extra: {'phone': requestPhone},
|
||||||
|
);
|
||||||
final message = e is ApiException
|
final message = e is ApiException
|
||||||
? e.message
|
? e.message
|
||||||
: L10n.current.authSendCodeFailed;
|
: L10n.current.authSendCodeFailed;
|
||||||
@@ -176,10 +184,11 @@ class LoginCubit extends Cubit<LoginState> {
|
|||||||
}
|
}
|
||||||
emit(state.copyWith(status: FormzSubmissionStatus.success));
|
emit(state.copyWith(status: FormzSubmissionStatus.success));
|
||||||
return response;
|
return response;
|
||||||
} catch (e) {
|
} catch (e, stackTrace) {
|
||||||
if (isClosed) {
|
if (isClosed) {
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
_logger.error(message: 'Login failed', error: e, stackTrace: stackTrace);
|
||||||
final message = e is ApiException ? e.message : e.toString();
|
final message = e is ApiException ? e.message : e.toString();
|
||||||
emit(
|
emit(
|
||||||
state.copyWith(
|
state.copyWith(
|
||||||
|
|||||||
Reference in New Issue
Block a user