react-native: fix breadcrumb file rotation never triggering on exact chunk boundaries

Author: KishanPRao

packages/react-native/src/storage/combinedChunkSplitter.ts

@@ -21,7 +21,8 @@ export function combinedChunkSplitter<W extends Chunk>(
         for (const splitter of splitters) {
             const [c1, c2] = splitter(chunk);
             chunk = c1;
-            if (c2) {
+            // An empty second chunk still means "split here"; dropping it loses the split.
+            if (c2 !== undefined) {
                 // Prepend second chunk to the rest
                 rest.unshift(c2);
             }

packages/react-native/tests/nativeBreadcrumbPropagationTests.spec.ts

@@ -53,8 +53,10 @@ function breadcrumbPathsSentToNative(): string[] {
         .filter((p: string) => p.includes('breadcrumb'));
 }
 
-function attributesSentToNative(): Record<string, string> {
-    return Object.assign({}, ...nativeMock.useAttributes.mock.calls.map((call) => call[0]));
+async function settle() {
+    for (let i = 0; i < 10; i++) {
+        await nextTick();
+    }
 }
 
 describe('BacktraceClient native breadcrumb propagation', () => {
@@ -67,21 +69,24 @@ describe('BacktraceClient native breadcrumb propagation', () => {
     it('Should tell the native crash reporter about the breadcrumb files created after rotation', async () => {
         const client = createClient();
         client.initialize();
-
-        const pathsAtInit = nativeMock.initialize.mock.calls[0][3].filter((p: string) => p.includes('breadcrumb'));
         nativeMock.useAttachments.mockClear();
 
         for (let i = 0; i < 20; i++) {
             client.breadcrumbs?.info(`breadcrumb-${i}`);
             await nextTick();
         }
+        await settle();
 
         const sentLater = breadcrumbPathsSentToNative();
-        expect(sentLater.length).toBeGreaterThan(0);
-        expect(sentLater.some((p) => !pathsAtInit.includes(p))).toBe(true);
+        expect(sentLater.some((p) => /bt-breadcrumbs-[1-9]/.test(p))).toBe(true);
+
+        const calls = nativeMock.useAttachments.mock.calls;
+        const lastPaths = calls[calls.length - 1][0].filter((p: string) => p.includes('breadcrumb'));
+        expect(lastPaths.length).toBeGreaterThan(0);
+        expect(lastPaths.some((p: string) => p.includes('bt-breadcrumbs-0_'))).toBe(false);
     });
 
-    it('Should send the last breadcrumb id to the native crash reporter', async () => {
+    it('Should send a fresh last breadcrumb id to the native crash reporter on every rotation', async () => {
         const client = createClient();
         client.initialize();
         nativeMock.useAttributes.mockClear();
@@ -90,7 +95,13 @@ describe('BacktraceClient native breadcrumb propagation', () => {
             client.breadcrumbs?.info(`breadcrumb-${i}`);
             await nextTick();
         }
-
-        expect(attributesSentToNative()['breadcrumbs.lastId']).toBeDefined();
+        await settle();
+
+        const pushed = nativeMock.useAttributes.mock.calls
+            .map((call) => call[0]['breadcrumbs.lastId'])
+            .filter((value) => value !== undefined)
+            .map(Number);
+        expect(pushed.length).toBeGreaterThan(1);
+        expect(pushed[pushed.length - 1]).toBeGreaterThan(pushed[0]);
     });
 });

packages/react-native/tests/storage/combinedChunkSplitter.spec.ts

@@ -0,0 +1,36 @@
+import { combinedChunkSplitter } from '../../src/storage/combinedChunkSplitter';
+import { lengthChunkSplitter } from '../../src/storage/lengthChunkSplitter';
+import { lineChunkSplitter } from '../../src/storage/lineChunkSplitter';
+
+const join = (chunks: string[]) => chunks.join('');
+
+describe('combinedChunkSplitter', () => {
+    it('should split when any splitter splits', () => {
+        const splitter = combinedChunkSplitter(join, lengthChunkSplitter(4), lineChunkSplitter(10));
+
+        const [c1, c2] = splitter('abcdefgh');
+
+        expect(c1).toEqual('abcd');
+        expect(c2).toEqual('efgh');
+    });
+
+    it('should not split when no splitter splits', () => {
+        const splitter = combinedChunkSplitter(join, lineChunkSplitter(10), lengthChunkSplitter(1000));
+
+        const [c1, c2] = splitter('line-1\n');
+
+        expect(c1).toEqual('line-1\n');
+        expect(c2).toBeUndefined();
+    });
+
+    it('should keep the split when the second chunk is empty', () => {
+        const splitter = combinedChunkSplitter(join, lineChunkSplitter(2), lengthChunkSplitter(1000, 'skip'));
+
+        splitter('line-1\n');
+        const [c1, c2] = splitter('line-2\n');
+
+        expect(c1).toEqual('line-2\n');
+        expect(c2).toBeDefined();
+        expect(c2?.length).toEqual(0);
+    });
+});