Merge branch 'main' into alwx/refactor/migrate-private-sentry-sdk-only

Author: alwx

.github/pull_request_template.md

@@ -7,7 +7,7 @@
 
 
 ## :scroll: Description
-<!--- Describe your changes in detail -->
+<!--- Describe your changes in detail and optionally add screenshots -->
 
 
 ## :bulb: Motivation and Context
@@ -16,6 +16,10 @@
 
 
 ## :green_heart: How did you test it?
+<!---
+Include a link to Sentry when applicable:
+* Link to Sentry: <LINK>
+-->
 
 
 ## :pencil: Checklist

CHANGELOG.md

@@ -14,6 +14,8 @@
 
 ### Fixes
 
+- Fix iOS time-to-initial-display fallback spans reporting a spurious `deadline_exceeded` status and inflated duration ([#6438](https://github.com/getsentry/sentry-react-native/pull/6438))
+- Fix `TypeError` when `showFeedbackForm`/`showFeedbackButton`/`showScreenshotButton` is called before `FeedbackFormProvider` mounts ([#6435](https://github.com/getsentry/sentry-react-native/pull/6435))
 - Fix orphaned TTID/TTFD spans in the trace view ([#6437](https://github.com/getsentry/sentry-react-native/pull/6437))
 
 ### Internal

packages/core/RNSentryCocoaTester/RNSentryCocoaTesterTests/RNSentryTimeToDisplayTests.swift

@@ -30,6 +30,29 @@ final class RNSentryTimeToDisplayTests: XCTestCase {
         XCTAssertNotNil(newestEntry)
     }
 
+    func testGetTimeToDisplayResolvesWithNumberNotArray() {
+        let sut = RNSentryTimeToDisplay()
+        let expectation = self.expectation(description: "resolve block is called")
+        var resolvedValue: Any?
+
+        // `getTimeToDisplay:` is invoked with an `RCTPromiseResolveBlock` from the
+        // Promise-based `getNewScreenTimeToDisplay` bridge method, so it must resolve
+        // with a raw timestamp value and never wrap it in an array.
+        sut.getTimeToDisplay { value in
+            resolvedValue = value
+            expectation.fulfill()
+        }
+
+        waitForExpectations(timeout: 5)
+
+        XCTAssertFalse(resolvedValue is [Any],
+                       "getTimeToDisplay must resolve with a scalar timestamp, not an array")
+        let number = try? XCTUnwrap(resolvedValue as? NSNumber,
+                                    "getTimeToDisplay must resolve with an NSNumber timestamp")
+        XCTAssertNotNil(number)
+        XCTAssertGreaterThan(number?.doubleValue ?? 0, 0)
+    }
+
     func testHandlesEarlyPoppedValues() {
         let maxSize = TIME_TO_DISPLAY_ENTRIES_MAX_SIZE + 1
         for i in 1...maxSize {

packages/core/ios/RNSentryTimeToDisplay.h

@@ -15,6 +15,6 @@ extern const int TIME_TO_DISPLAY_ENTRIES_MAX_SIZE;
 + (void)setActiveSpanId:(NSString *)spanId;
 + (void)putTimeToInitialDisplayForActiveSpan:(NSNumber *)timestampSeconds;
 
-- (void)getTimeToDisplay:(RCTResponseSenderBlock)callback;
+- (void)getTimeToDisplay:(RCTPromiseResolveBlock)callback;
 
 @end

packages/core/ios/RNSentryTimeToDisplay.m

@@ -8,7 +8,7 @@
 // React Native bridge / JS thread (setActiveSpanId, pop). Synchronize every access.
 @implementation RNSentryTimeToDisplay {
     CADisplayLink *displayLink;
-    RCTResponseSenderBlock resolveBlock;
+    RCTPromiseResolveBlock resolveBlock;
 }
 
 static NSMutableDictionary<NSString *, NSNumber *> *screenIdToRenderDuration;
@@ -94,7 +94,7 @@ + (void)putTimeToDisplayFor:(NSString *)screenId value:(NSNumber *)value
 }
 
 // Rename requestAnimationFrame to getTimeToDisplay
-- (void)getTimeToDisplay:(RCTResponseSenderBlock)callback
+- (void)getTimeToDisplay:(RCTPromiseResolveBlock)callback
 {
     // Store the resolve block to use in the callback.
     resolveBlock = callback;
@@ -104,7 +104,7 @@ - (void)getTimeToDisplay:(RCTResponseSenderBlock)callback
     displayLink = [CADisplayLink displayLinkWithTarget:self selector:@selector(handleDisplayLink:)];
     [displayLink addToRunLoop:[NSRunLoop mainRunLoop] forMode:NSRunLoopCommonModes];
 #else
-    resolveBlock(@[]); // Return nothing if not iOS.
+    resolveBlock(nil); // Return nothing if not iOS.
 #endif
 }
 
@@ -116,7 +116,7 @@ - (void)handleDisplayLink:(CADisplayLink *)link
 
     // Ensure the callback is valid and pass the current time back
     if (resolveBlock) {
-        resolveBlock(@[ @(currentTime) ]); // Call the callback with the current time
+        resolveBlock(@(currentTime)); // Resolve the promise with the current time
         resolveBlock = nil; // Clear the block after it's called
     }
 

packages/core/src/js/feedback/FeedbackFormManager.tsx

@@ -19,7 +19,9 @@ const NOOP_SET_VISIBILITY = (): void => {
 
 abstract class FeedbackManager {
   protected static _isVisible = false;
-  protected static _setVisibility: (visible: boolean) => void;
+  // Default to NOOP so `show()`/`hide()` don't call `undefined(...)` when
+  // invoked before `FeedbackFormProvider` mounts and calls `initialize()`.
+  protected static _setVisibility: (visible: boolean) => void = NOOP_SET_VISIBILITY;
 
   protected static get _feedbackComponentName(): string {
     throw new Error('Subclasses must override feedbackComponentName');

packages/core/test/feedback/FeedbackFormManager.test.tsx

@@ -87,6 +87,29 @@ describe('FeedbackFormManager', () => {
     }).not.toThrow();
   });
 
+  it('showFeedbackForm does not throw before any provider or reset has run', () => {
+    // Regression for SDK-CRASHES-REACT-NATIVE-62C: `_setVisibility` used to be
+    // uninitialized, so calling show() before FeedbackFormProvider mounted threw
+    // `TypeError: this._setVisibility is not a function`. Load the module fresh
+    // so the class statics have never been touched by other tests.
+    jest.isolateModules(() => {
+      // eslint-disable-next-line @typescript-eslint/no-require-imports
+      const freshManager = require('../../src/js/feedback/FeedbackFormManager');
+      expect(() => freshManager.showFeedbackForm()).not.toThrow();
+      expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining("requires 'Sentry.wrap(RootComponent)'"));
+    });
+  });
+
+  it('hideFeedbackForm does not throw before any provider or reset has run', () => {
+    // Symmetric to show(): hide() reads the same uninitialized static field.
+    jest.isolateModules(() => {
+      // eslint-disable-next-line @typescript-eslint/no-require-imports
+      const freshManager = require('../../src/js/feedback/FeedbackFormManager');
+      expect(() => freshManager.FeedbackFormManager.hide()).not.toThrow();
+      expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining("requires 'Sentry.wrap(RootComponent)'"));
+    });
+  });
+
   it('showFeedbackForm displays the form with the feedbackIntegration options', async () => {
     mockedIsModalSupported.mockReturnValue(true);
     const { getByPlaceholderText, getByText } = render(