Skip to content

refactor: centralize Firebase logging in HTTP interceptor#159

Draft
rileychh wants to merge 3 commits intomainfrom
centralize-firebase-http-logging
Draft

refactor: centralize Firebase logging in HTTP interceptor#159
rileychh wants to merge 3 commits intomainfrom
centralize-firebase-http-logging

Conversation

@rileychh
Copy link
Member

@rileychh rileychh commented Mar 4, 2026

Summary

Instead of sprinkling _firebase.log() calls in individual services, log all HTTP calls automatically via LogInterceptor.

  • LogInterceptor now logs to both dart:developer and Crashlytics — every HTTP request/response becomes a breadcrumb automatically
  • FirebaseService is now a top-level const instead of a Riverpod provider (it's stateless — just a facade over static Firebase singletons)
  • PortalService no longer takes FirebaseService as a constructor dependency
  • Body length uses type-safe switch expressions instead of dynamic .length dispatch, fixing potential NoSuchMethodError for FormData/Stream bodies

Trade-offs

  • We lose semantic breadcrumbs like "Login successful" — Crashlytics will show POST https://app.ntut.edu.tw/login.do => 200 text/html 1KB instead
  • We gain automatic coverage of every HTTP call without any per-service wiring
  • New services get Crashlytics breadcrumbs for free without any setup

Test plan

  • dart analyze passes
  • Integration tests pass (pre-existing data-dependent failures unrelated to this change)

Move Crashlytics breadcrumb logging from PortalService into
LogInterceptor so all HTTP calls are automatically logged.
FirebaseService becomes a top-level constant, removing the
Riverpod provider and simplifying service constructors.
@rileychh-dokploy-riley-ntut-npc
Copy link

rileychh-dokploy-riley-ntut-npc bot commented Mar 4, 2026

Dokploy Preview Deployment

Name Status Preview Updated (UTC)
API Docs ✅ Done Preview URL 2026-03-04T18:25:30.712Z

@github-actions
Copy link

github-actions bot commented Mar 4, 2026

PR Preview Builds

Build Number: 514
Commit: ec80a43
Message: fix: use type-safe body length in HTTP log interceptor

Deploy

  • Android (Firebase App Distribution)
  • iOS (TestFlight)

Android (Firebase App Distribution)

Install build 514

iOS (TestFlight)

  1. Open the TestFlight app on your iOS device
  2. Select "Project Tattoo"
  3. Install build 514

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR centralizes Firebase Crashlytics breadcrumb logging for network traffic by emitting HTTP request/response logs from the shared Dio LogInterceptor, and removes FirebaseService as an injected dependency from services in favor of a global firebase facade.

Changes:

  • Log all HTTP responses to both dart:developer and Firebase Crashlytics via LogInterceptor.
  • Replace the Riverpod firebaseServiceProvider with a global const firebase instance.
  • Simplify PortalService construction and update affected integration tests and router creation accordingly.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/services/student_query_service_test.dart Updates PortalService construction after removing FirebaseService injection.
test/services/portal_service_test.dart Updates PortalService construction after removing FirebaseService injection.
test/services/i_school_plus_service_test.dart Updates PortalService construction after removing FirebaseService injection.
test/services/course_service_test.dart Updates PortalService construction after removing FirebaseService injection.
lib/utils/http.dart Adds Crashlytics breadcrumb logging for HTTP responses via firebase.log().
lib/services/portal_service.dart Removes FirebaseService dependency and per-method Firebase logging.
lib/services/firebase_service.dart Removes Riverpod provider and introduces global const firebase facade.
lib/router/app_router.dart Updates router factory to use global firebase.analyticsObserver instead of injected FirebaseService.
lib/main.dart Stops reading FirebaseService from Riverpod; uses global firebase and updates router creation call.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@rileychh rileychh requested a review from bradly0cjw March 4, 2026 18:25
@bradly0cjw
Copy link
Member

PR Summary

This PR refactors the network logging architecture by centralizing Firebase Crashlytics breadcrumbs into a single HTTP interceptor and streamlining the dependency injection graph.

Key Changes & Fixes:

  • Centralized HTTP Logging: Moved _firebase.log() calls from individual services (like PortalService) into a shared Dio LogInterceptor. Every HTTP request and response is now automatically logged to both dart:developer and Firebase Crashlytics.
  • Type-Safe Body Length Calculation: Fixed a potential runtime crash (NoSuchMethodError) when logging HTTP requests by replacing dynamic .length calls with a type-safe switch expression, which now safely handles FormData and Stream bodies.
  • Simplified Dependency Injection: Removed FirebaseService from Riverpod providers. Since it acts as a stateless facade over static Firebase singletons, it's now defined as a top-level const firebase instance.
  • Constructor Cleanup: Stripped the FirebaseService dependency from PortalService and other service constructors, significantly reducing boilerplate.

Architectural Impact:
This eliminates the "shotgun surgery" code smell—new services will now get Crashlytics breadcrumbs for free without any manual wiring. It also cleans up the DI container by removing unnecessary wrapper providers and eliminates a potential point of failure with dynamic type dispatching.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants