Repository navigation
fix(firebase_crashlytics_tvos): port to firebase_crashlytics 5.3.0 (Swift/Pigeon), 0.0.2 - #28
TheNoumanDev wants to merge 2 commits into
Conversation
…wift/Pigeon), 0.0.2 firebase_crashlytics 5.3.0 moved the Apple plugin from a MethodChannel to Pigeon and from ObjC to Swift. 0.0.1 only answered the old channel, so with 5.3.0 every call on tvOS failed with channel-error. The native code is now upstream 5.3.0's Swift plugin, generated Pigeon messages and ObjC helpers, with three tvOS edits marked `// tvOS:`: import firebase_core_tvos, and allow tvOS in two os(iOS) checks. pluginClass is now FirebaseCrashlyticsPlugin. Verified on the tvOS 27.0 simulator: every Crashlytics call returns without error, and a forced crash is reported on the next launch.
|
@DenisovAV @MAUstaoglu Please take a look, thanks |
DenisovAV
left a comment
There was a problem hiding this comment.
Approve. The port is faithful where it says it is, and the premise holds.
The port is verbatim
Every ported file diffed against firebase_crashlytics 5.3.0's ios/firebase_crashlytics/Sources/:
Constants.swift,CrashlyticsPlatformHelpers.m,include/CrashlyticsPlatformHelpers.h,include/Crashlytics_Platform.h,include/ExceptionModel_Platform.h— byte-identical. The two lines dropped fromCrashlytics_Platform.hmove it to upstream, not away from it.FirebaseCrashlyticsMessages.g.swift— one hunk, the#ifthe report names.FirebaseCrashlyticsPlugin.swift— two hunks, the two the report names.
Three edits claimed, three found, nothing else in 930 added lines.
The channel-error premise
5.3.0's Dart side talks on ten channels only, all dev.flutter.pigeon.firebase_crashlytics_platform_interface.FirebaseCrashlyticsHostApi.*, and the ten the new .g.swift registers are that same set exactly. plugins.flutter.io/firebase_crashlytics — the one channel 0.0.1 answered — appears nowhere in the platform interface's call path.
This is live on pub rather than hypothetical: published 0.0.1 declares firebase_crashlytics: ^5.2.4, which resolves to 5.4.0 today, so every fresh install of 0.0.1 is already in that state.
The chain a rename could have broken
FirebaseCrashlytics._ passes 'plugins.flutter.io/firebase_crashlytics' to FirebasePlugin; the platform interface asserts pluginConstants['isCrashlyticsCollectionEnabled'] != null; firebase_core_tvos's registry keys that map by each plugin's flutterChannelName. The Swift still returns that exact string and still implements pluginConstants(for:), so the startup assert is satisfied.
The Swift↔ObjC names work out against the tvOS core specifically: firebase_core_tvos declares both @protocol FLTFirebasePlugin and @interface FLTFirebasePlugin, so Swift sees the protocol as FLTFirebasePluginProtocol; registerFirebasePlugin: arrives as register(_:), pluginConstantsForFIRApp: as pluginConstants(for:). All five @required members are implemented.
Pods, versions, resolution
Firebase/Crashlytics ~> 12.15.0 agrees with firebase_core_tvos's Firebase/CoreOnly ~> 12.15.0, and the report's "upstream uses 12.18.0" matches firebase_core 4.14.0's firebase_sdk_version.rb. Dropping the LIBRARY_NAME/LIBRARY_VERSION defines is right — nothing reads them now. pubspec 0.0.2 = podspec 0.0.2 = ## 0.0.2.
The README's platform-interface sentence names a narrower pairing than the dependency requires, but is safe either way: firebase_core_platform_interface 8.1.0 and 8.1.1 generate byte-identical Pigeon Dart, as do firebase_crashlytics_platform_interface 3.9.0 and 3.9.1, so firebase_core_tvos 0.0.3 (regenerated for 8.1.0) is valid on the 8.1.1 the example resolves.
Locally: flutter pub get resolves for the package and the example, dart analyze is clean on both, the test passes.
Findings — none blocking
1. ^5.3.0 is the right bound; worth recording why. The package resolves firebase_crashlytics 5.4.0 today, not 5.3.0, and that is safe: 5.3.0, 5.4.0 and 6.0.0 ship byte-identical ios/.../Sources apart from versionNumber, and all three pin firebase_crashlytics_platform_interface: ^3.9.0. 6.0.0 is excluded by the caret and would have to be — it requires firebase_core: ^5.0.0, which firebase_core_tvos 0.0.3 (firebase_core: ^4.13.0) cannot satisfy. A pubspec comment saying the window is verified rather than assumed would stop the next reader tightening it needlessly or widening it on the next major.
2. The whole class has a gate that runs on Linux. What broke here is a set mismatch between the dev.flutter.pigeon.* names in tvos/Classes/*.g.swift and those in the resolved platform interface's lib/src/pigeon/messages.pigeon.dart. After pub get both are text in the tree, so comparing the two sets is a Dart test or a sixth check_repo.py rule, and it would have failed 0.0.1 against 5.3.0 with no Apple hardware involved. R1–R5 read no native sources today: R5 checks that a pluginClass is declared, not that a type by that name exists under tvos/ — the other half of this PR's rename, and the same kind of string comparison. This family has paid for the gap three times now (firebase_core_tvos 0.0.2's 15-vs-14-element payload, firebase_app_check_tvos #26, and this).
3. The green check on this package is thinner than it looks. test/firebase_crashlytics_tvos_test.dart is still the generated expect(1 + 1, 2), so the job proves resolution and analysis only — by design, per the workflow header, which is what makes the PR's testing box the only evidence for the native half.
4. A device run is the one claim 0.0.2 cannot make and 0.0.1 could. The README table is honest about it. For a crash reporter it is also the claim that matters most: the simulator exercises the Pigeon surface, while dSYM handling and the signal/NSException path on real hardware are what the package exists for. Worth closing before publishing 0.0.2 to pub.
5. versionNumber will not track the resolved package. It is what the native side reports through FIRApp registerLibrary:withVersion:, so an app on 5.4.0 reports 5.3.0. Cohort-consistent, and already drifted in a sibling: firebase_analytics_tvos 0.0.1 ships versionNumber = "11.4.6" against firebase_analytics: ^12.4.3.
6. The Firebase library identity changes and the changelog doesn't say so. 0.0.1 reported flutter-fire-cls-tvos / 0.0.1 from the podspec defines; 0.0.2 reports upstream's flutter-fire-cls / 5.3.0. That matches the five already-migrated siblings — fn, analytics, appcheck, perf, rc all dropped the suffix — so it is the right call, but the four still-ObjC packages (core, auth, fst, fcm) keep -tvos, so what the fork tells Firebase is now split down the middle. One changelog line would record the decision rather than leave it to be rediscovered.
Not this PR
CI is red on sqflite_tvos, which fails in pub get because the workflow pins FLUTTER_VERSION: 3.44.0 while that package needs 3.47.7; the ci job only mirrors the matrix.
The larger thing this port surfaces: firebase_crashlytics 6.0.0 and firebase_core 5.0.0 both landed on 15 October, five days before this PR, and 6.0.0 requires the 5.x core train. Nothing here is affected, but the tvOS family is now a major behind at the Dart level, and the gate on catching up is firebase_core_tvos against core 5.0.0 — which pins Firebase Apple SDK 12.19.0 where the tvOS pods pin 12.15.0.
|
Hey @DenisovAV Thanks for the detailed review! Added in 3718029:
(4) Tested on a real device: Apple TV 4K (3rd gen), tvOS 27.0, release build of the example. The app starts, and the test crash goes through the NSException path ( (2) and (3) seem better as separate repo-wide changes, so I've left them out of this PR. (5) stays as is, the same as the other ports. |
What does this PR do?
firebase_crashlytics5.3.0 moved its Apple plugin to Swift + Pigeon. 0.0.1 still answers the old MethodChannel, so with 5.3.0 every call on tvOS fails withchannel-error.The native code is now upstream 5.3.0, with three tvOS edits (marked
// tvOS:): importfirebase_core_tvos, and allowos(tvOS)in twoos(iOS)checks.pluginClassis nowFirebaseCrashlyticsPlugin.Package(s) touched: firebase_crashlytics_tvos
How was it tested?
example/appcrash()is detected on relaunch. With 0.0.1 they fail withchannel-error.dart analyzeis clean