Repository navigation
[merge after iOS SDK 7.0.0-beta7] logException: domain only, as in the 7.x SDKs (no labels, no includeVideo) - #89
Closed
dsheikherev wants to merge 4 commits into
Closed
dsheikherev wants to merge 4 commits into
dsheikherev wants to merge 4 commits into
Conversation
Neither 7.x SDK's logException takes labels or can leave the video out of
one report any more (bugsee-cocoa#233, bugsee-android#256). JS
ExceptionOptions is {domain?}; labels and includeVideo are unknown keys
and throw TypeError, like any other. A report is labelled in
setReportHandler.
e2e: the labels cases (exceptions case 7, [API-13d]) are gone, [API-11d]
checks that includeVideo is refused and files nothing, and the scenarios
stop passing labels. Playground: the includeVideo and labels actions and
their known issues are gone; the shop and the boundary demo use a domain.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
7.0.0-beta7 removes BugseeExceptionLoggingOptions and the options:
overloads (bugsee-cocoa#233). The bridge parses {domain?} into its own
BGSRNExceptionOptions, ignoring any other key, and calls
logException:reason:domain:skipFrames:completion: with skipFrames 0: it
leaves top native frames out of the report's signature, and an RN report
is keyed on the JS signature in the payload. The e2e checks that the
report's exception options no longer carry includeVideo.
Comments only: beta7 accepts logException while the SDK is Launching
(bugsee-cocoa#234), so BGSRNLaunchHold now holds it only for the order of
calls and for createReport/showReportDialog, which still need Launched;
the simulator calls logUnhandledException's completion (bugsee-cocoa#232),
and BGSRNSettleOnce stays so a promise settles exactly once.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
bugsee-android#256 deprecates logException(Throwable, Map), whose labels
and includeVideo were never applied, for logException(Throwable, String
domain, int skipFrames). The bridge parses {domain?} and calls the new
overload with skipFrames 0.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n APIs
§9.1 names the iOS 7.0.0-beta7 and Android #256 overloads the bridges
call, and ExceptionOptions is {domain?}; the Removed list adds
logException's labels and includeVideo options.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 25.93% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 27 functions across 21 files. (4 skipped:… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly summarizes the main change: adapting logException to the iOS SDK 7.0.0-beta7 and 7.x domain-only API while removing labels and includeVideo.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 25.93% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 27 functions across 21 files. (4 skipped: 4 unsupported.)
- Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
dsheikherev
marked this pull request as ready for review
October 14, 2026 17:18
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Deep review — b96d834
Intent is correct: ExceptionOptions becomes {domain?} to match the converged 7.x logException APIs (cocoa#233, android#256). Labels and includeVideo are unknown keys (TypeError, nothing crosses). Both bridges pass the domain and skipFrames 0. The package is still unpublished (0.0.0).
Reviewed against callers (Bugsee.logException, ErrorBoundary, reportHandled), the 7.4.1 Android javadoc, cocoa#233/#234 notes, BGSRNLaunchHold, e2e/playground fallout, and CI on this SHA. Diff was not reviewed in isolation.
Findings
No P0–P3 defects in the unique code.
Adversarial checks that did not land as defects:
- Null domain on the Android 3-arg API.
ExceptionBridgealways callslogException(t, domain, 0)withdomain == nullwhen JS sent no options. 7.4.1 javadoc marks thatString@Nullable(“May be null”). Negatives clamp to 0. CIandroidand playground Android build are green on this SHA — 7.4.1 already has the overload (it shipped in 7.4.0). The PR body’s “AndroidassembleDebugcannot compile until a post-7.4.1 release” is stale; only iOS is blocked on a pin. skipFrames0. Android javadoc: skipFrames is “how many top frames of the exception’s own stack to leave out of the report’s signature.” 0 includes every native frame ofReactNativeWebException(constructed at one bridge site). That matches the old Map path with skipFrames absent. RN grouping is the JS payload (ReactNativeWebException+ worker); skipFrames cannot uniquify JS errors anyway.- Malformed / empty options. JS encodes
undefined/{}asnull. Native: nil JSON → no domain;{}→ Options with nil domain; bad JSON / wrong-typed domain → log and still send. iOSopts.domainon a niloptsis messaging-nil. Same never-throw contract as before. - Labels /
includeVideoTypeError. Matches the existing unknown-key policy. TypeScript rejects the old fields.ErrorBoundaryalready reports without options when encode throws, so a leftoverlabelson the boundary drops the domain too — same as any other bad option, and this wrapper is0.0.0. Replacement path issetReportHandler(Report.addLabels); that is what 7.4.1 javadoc and cocoa#233 specify. BGSRNLaunchHoldstill queueslogExceptionduringLaunchingafter cocoa#234. Intentional: keeps order withcreateReport/showReportDialog, which still needLaunched. Drop-on-failed-launch is the same window as today. The “sent” NSLog still requiresLaunched; the hold still prevents the Launching path, so e2eexception handled sentandearly-reportstay consistent.
Merge-order residual (not a code defect)
native-versions.json still pins iOS 7.0.0-beta6. BugseeModule.mm calls logException:reason:domain:skipFrames:completion:, which is 7.0.0-beta7 (cocoa#233; #232/#234 under/over it). BugseeRNSupport no longer references SDK option types — iOS unit tests pass against beta6. iOS SPM / playground iOS build are red until the pin moves. Do not merge before that.
Device e2e was not run (author). JS lint/typecheck/unit and Android CI were green on this SHA; I did not re-run them locally (no node_modules in this environment).
- Overall risk: Low (unique logic). iOS app compile is merge-order, not a logic bug.
- Merge recommendation: Approve after the iOS pin moves to 7.0.0-beta7. Keep stacked on #81. Android does not need to wait for a later SDK.
- Most important to fix before merge: Bump the iOS pin (and only then land). Nothing else in this tree.
- Positive: JS / iOS Support / Android bridge stay in lockstep; unknown keys are ignored natively so a future JS field cannot refuse a report; playground, e2e, and design §9.1 were updated with the API rather than left asserting removed behaviour.
Sent by Cursor Automation: Bugsee code review
dsheikherev
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adapts the wrapper to the 7.x
logExceptionAPIs, now the same shape on both platforms: a domain andskipFrames, no labels, noincludeVideo.Changes
feat(exceptions)!):ExceptionOptionsis{domain?}.labelsandincludeVideoare unknown keys and throwTypeError; a report is labelled insetReportHandler. The wrapper is unpublished (0.0.0).[API-13d]);[API-11d]checks thatincludeVideois refused and files nothing (runForwaits for 3 bundles, not 5); the scenarios stop passing labels (staging finds its report by the campaign text, not by labels).includeVideoand labels actions and the cocoa-226 / android-247 known issues are gone; the shop and the boundary demo use a domain.fix(ios)):BGSRNExceptionsparses{domain?}intoBGSRNExceptionOptions;BugseeModule.mmcallslogException:reason:domain:skipFrames:completion:withskipFrames0. Comments: beta7 acceptslogExceptionwhile Launching (cocoa#234), soBGSRNLaunchHoldstays only for call order and forcreateReport/showReportDialog; the simulator now callslogUnhandledException's completion (cocoa#232),BGSRNSettleOncestays. e2e asserts the report's exception options carry noincludeVideo.fix(android)):ExceptionBridgeparses{domain?}and callslogException(Throwable, String, int)withskipFrames0.skipFramesis always 0: it leaves top native frames out of the report's signature, and an RN report is keyed on the JS signature in the payload.Verification (local)
yarn lint,yarn typecheck,yarn test: 3577 passed.BugseeRNSupport(xcodebuild, iOS 26.5 simulator): 322/0 — the Support package no longer references SDK option types, so it builds against beta6 too.:bugsee-android-bridge+:bugsee-android-feedbackunit tests: pass (BugseeModule.javais excluded there).Bugsee.xcframeworkbuilt from the SDK stack tip (cocoafix/log-exception-while-launching):BugseeModule.mmcompiles and links, and the binary callslogException:reason:domain:skipFrames:completion:. Only the JS bundle phase failed (nocredentials.jsonwithoutBUGSEE_TOKEN_IOS).android(incl.assembleDebug, soBugseeModule.javaagainst 7.4.1),ios unit (BugseeRNSupport), lint/typecheck/unit, mutation, pack install — pass; the four iOS build jobs fail only on the beta7 call above.🤖 Generated with Claude Code
Summary by CodeRabbit
logExceptionnow accepts only the optionaldomainsetting. Calls that includelabelsorincludeVideoare no longer supported.setReportHandlerto apply labels to reports.