[#16044] Crashlytics Error Identity for Non-Fatals

[yakovmanshin]

• Introduced the FIRCLSErrorInspector utility class to describe the identity of non-fatal errors reported to Crashlytics
• Updated -[FIRCrashlytics recordError:userInfo:] to report such descriptions with the additional user info
• Moved FIRCLSwiftFileUtility into the new SwiftUtilities folder inside Crashlytics

See the detailed proposal in #16044.

[gemini-code-assist]

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

[yakovmanshin]

For this error enum:

enum TestError: Error {
    case errorCase1
    case errorCase2(Int)
}

…the result looks like this:

Notice how nserror-codes don’t follow the order of declaration.

[morganchen12]

Thanks for the reference implementation. I commented on one potential concern, otherwise this proposal and implementation (broadly) LGTM.

[morganchen12]

FYI, this may get flagged by Apple's private symbol scanner either now or in a future version of their scanner. We had a similar check for __SwiftValue in Analytics that was fine for several years and then caused App Store submission issues suddenly when Apple updated their scanner. Is there a way to implement this that doesn't require checking for the presence of a private class?

[yakovmanshin]

My original implementation relied on a combination of isKind(of:) and isMember(of:), but it couldn’t distinguish between bridged Swift errors and subclasses of the real NSError (e.g. @interface MyError : NSError) because both are “kind of” but not “member of” NSError.

The best way I can think of to avoid issues with App Store’s static analysis is to reduce granularity in the check: e.g. something more generic, like SwiftNative, seems unlikely to be on the watch list of private APIs.

[yakovmanshin]

The error in podspec validation should be fixed by ce52129, but I’m still not sure what’s causing Catalyst tests to fail:

Executed 208 tests, with 0 failures (0 unexpected) in 6.431 (7.245) seconds
2026-04-07 17:13:40.336 xcodebuild[28476:81604] [MT] IDETestOperationsObserverDebug: 18.650 elapsed -- Testing started completed.
2026-04-07 17:13:40.336 xcodebuild[28476:81604] [MT] IDETestOperationsObserverDebug: 0.000 sec, +0.000 sec -- start
2026-04-07 17:13:40.336 xcodebuild[28476:81604] [MT] IDETestOperationsObserverDebug: 18.650 sec, +18.650 sec -- end
** TEST FAILED **

[yakovmanshin]

/gemini review

[gemini-code-assist]

Code Review

This pull request adds an ErrorInspector utility to provide standardized error identity descriptions and integrates it into FIRCrashlytics. Feedback identifies a critical missing nil check that could cause a runtime crash and suggests using more idiomatic Objective-C syntax for dictionary manipulation.

[yakovmanshin]

Podspec validation now passes, and the Catalyst job failed for some unrelated reason:

Fetching kramdown-parser-gfm 1.1.0
Retrying download gem from https://rubygems.org/ due to error (2/4): Gem::RemoteFetcher::UnknownHostError no such name (https://rubygems.org/gems/kramdown-parser-gfm-1.1.0.gem)

Retrying download gem from https://rubygems.org/ due to error (3/4): Gem::RemoteFetcher::UnknownHostError no such name (https://rubygems.org/gems/kramdown-parser-gfm-1.1.0.gem)

Retrying download gem from https://rubygems.org/ due to error (4/4): Gem::RemoteFetcher::UnknownHostError no such name (https://rubygems.org/gems/kramdown-parser-gfm-1.1.0.gem)


Gem::RemoteFetcher::UnknownHostError: no such name
(https://rubygems.org/gems/kramdown-parser-gfm-1.1.0.gem)
An error occurred while installing kramdown-parser-gfm (1.1.0), and Bundler
cannot continue.

Just in case, I merged the latest commits from main.

@morganchen12 Could you please start the CI workflow? Thanks!

[paulb777]

I restarted CI. The catalyst job can be flaky.

[yakovmanshin]

Seems that all checks are fine now 🤞🏻

[morganchen12]

LGTM with one nit. Adding someone from Crashlytics to approve as well.

[morganchen12]

nit: if you can link to a reference for this and other private-ish behaviors, that would be good (if such a reference exists).

[yakovmanshin]

TBH they mostly come empirically from experiments in a playground and prototype apps, though there must be a precise explanation for each, of course.

For the enum behavior specifically, which seems to be the most questionable thing here, the direct reference I found is this:

1. The runtime reflection mechanism uses EnumImpl, a ReflectionMirrorImpl subclass;
2. To get the number of elements in the resulting Mirror.children array, EnumImpl’s count calls getInfo;
3. getInfo retrieves the index (tag) of the enum case and calls getFieldAt with the type (the enum itself) and the case index;
4. From getFieldAt’s return, getInfo extracts payloadType (which represents the case’s associated value) and assigns the pointer to the payloadTypePtr in-out parameter;
5. count checks this pointer and returns 1 if the enum case has a payload (associated value) and 0 if not.

(At least that’s what I could figure from this C++ code. Looks plausible, but C++ is not my domain.)

[tejasd]

@yakovmanshin Thanks for the FR and proposed implementation.

Based on looking at it, I'm not sure adding an additional key crashlytics_error_identity for all users' non-fatal errors (in addition to the existing nserror-code and nserror-domain keys) is the right implementation.

Some potential changes:

• only add that key in case of Swift errors (and rename it to swift-error)
• If you can verify that using ErrorInspector to make the error code and domain more consistent/deterministic for Swift errors, I can also take a second look to see if it fixes the clustering issue for Swift Errors that you mentioned.

However that's a larger effort, and would need more approvals to merge it into main.

[yakovmanshin]

@tejasd Thanks for the review!

My proposed implementation is a conservative one. It avoids modifying any existing data—but at the cost of reporting an extra key with every error. While it doesn’t fix error grouping automatically, it makes it possible to eventually update the backend to use the “error identity” as the new basis for grouping errors (once this concept is proven stable and consistent, as you mention in your second point).

Limiting the scope to only Swift errors (maybe even only Swift enums) is an easy tweak in the implementation. But to make the update helpful at scale, it would require branching in the backend logic: enums should be grouped based on one key, and all other errors, based on the old nserror-domain and nserror-code (though being able to identify individual enum errors, even if grouped incorrectly, is still a net positive; currently users don’t even have that).

Alternative approach (which I don’t like)

It’s a way larger project than even the proposed implementation. It streamlines the logic and reported errors’ format, avoids sending redundant data, and doesn’t require complex changes on the backend. But it basically comes down to switching away from NSError altogether, because enum errors fundamentally don’t fit the model with numeric error codes (even if we find a stable way to map cases to codes (the runtime assigns them in a random order), it’ll still break if the user as much as reorders cases in their error enums).

Conceptually, it would work like this:

• -[FIRCrashlytics recordError:userInfo:] inspects the reported error and repackages it into a new object (e.g. FIRCLSError) that has an “error identifier” (string):
  • For CustomNSError and real NSError, the identifier is the error domain and code;
  • For Swift enums, it’s the type name and case name;
  • For other Swift types (those that conform to Error but not CustomNSError), it’s just the type name (which would remove the MyApp.(unknown context at $…) part seen in the screenshots).
• userInfo (which is only available in NSError and CustomNSError) is copied as is;
• This new FIRCLSError is passed to FIRCLSUserLoggingWriteError where it’s written and reported, just as it happens now;
• The “error identifier” is used to group errors.

It’s a much bigger and riskier change, and frankly, I don’t like it. That’s why I proposed this conservative approach.

[tejasd]

@tejasd Thanks for the review!

My proposed implementation is a conservative one. It avoids modifying any existing data—but at the cost of reporting an extra key with every error. While it doesn’t fix error grouping automatically, it makes it possible to eventually update the backend to use the “error identity” as the new basis for grouping errors (once this concept is proven stable and consistent, as you mention in your second point).

Limiting the scope to only Swift errors (maybe even only Swift enums) is an easy tweak in the implementation. But to make the update helpful at scale, it would require branching in the backend logic: enums should be grouped based on one key, and all other errors, based on the old nserror-domain and nserror-code (though being able to identify individual enum errors, even if grouped incorrectly, is still a net positive; currently users don’t even have that).

Alternative approach (which I don’t like)

Makes sense, thanks.

If you can update this PR to add the swift-error custom key, I can take a second look and approve merging it into main.

For the clustering changes, like you mentioned this would be a larger effort - and we'll keep that in mind if we receive more feedback on the feature request.

[yakovmanshin]

@tejasd Sure! In ed458f3, I renamed the key to swift-error and updated ErrorInspector to ignore true NSErrors. Please have a look.

[tejasd]

Can you change this to only add the key if the swiftErrorIdentity exists?

[yakovmanshin]

From what I see, it already works like that: this method call returns an optional, and the next line only adds the key if the value is not nil.

Or did I not understand your comment?

[tejasd]

Please move this to be the first check.

[yakovmanshin]

@tejasd Could you have another look at this PR? Thanks!