Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 35 additions & 3 deletions client_sdks/devconnect-react-native/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,7 @@ export class DevConnect {
* own duplicate report if the count is > 0.
*/
private fetchInFlight: Map<string, number> = new Map();
private fetchStackCount = 0;

private constructor(config: DevConnectConfig & { resolvedHost: string }) {
this.config = {
Expand Down Expand Up @@ -920,8 +921,38 @@ export class DevConnect {
dc.fetchInFlight.set(fetchKey, (dc.fetchInFlight.get(fetchKey) ?? 0) + 1);
dc.send('client:network:request_start', { requestId, method, url, startTime, requestHeaders: reqHeaders, requestBody, source, via: 'fetch' });

let responsePromise: Promise<Response>;
dc.fetchStackCount++;
try {
const response = await originalFetch(input, init);
responsePromise = originalFetch(input, init);
} catch (err: any) {
dc.send('client:network:request_complete', {
requestId,
method,
url,
statusCode: 0,
startTime,
endTime: Date.now(),
duration: Date.now() - startTime,
requestHeaders: reqHeaders,
requestBody,
error: err?.message ?? String(err),
source,
via: 'fetch',
});
const currentCount = dc.fetchInFlight.get(fetchKey) ?? 0;
if (currentCount <= 1) {
dc.fetchInFlight.delete(fetchKey);
} else {
dc.fetchInFlight.set(fetchKey, currentCount - 1);
}
throw err;
} finally {
dc.fetchStackCount--;
}
Comment on lines +924 to +952

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

There are two issues with this implementation:

  1. Synchronous Error Handling Regression: If originalFetch throws an error synchronously (e.g., due to invalid arguments or synchronous validation in another interceptor), the error will propagate immediately and bypass the main catch block starting at line 953. This means the client:network:request_complete event will never be sent, leaving the request permanently in a pending state in the UI.
  2. Re-entrancy / Nested Fetches: If a nested fetch is triggered, the finally block of the nested fetch will set isInsideFetch to false prematurely, clearing the flag for the outer fetch.

We can resolve both issues by:

  • Wrapping the synchronous call to originalFetch so that any synchronous exceptions are captured as a rejected promise.
  • Using a counter (fetchStackCount) instead of a boolean flag.
      let responsePromise: Promise<Response>;
      dc.fetchStackCount++;
      try {
        responsePromise = Promise.resolve(originalFetch(input, init));
      } catch (err) {
        responsePromise = Promise.reject(err);
      } finally {
        dc.fetchStackCount--;
      }


try {
const response = await responsePromise;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
const clone = response.clone();
let responseBody: any;
try { const text = await clone.text(); try { responseBody = JSON.parse(text); } catch (_) { responseBody = text; } } catch (_) {}
Expand Down Expand Up @@ -968,6 +999,7 @@ export class DevConnect {

function PatchedXHR(this: any) {
const xhr = new OriginalXHR();
const isFetchXhr = dc.fetchStackCount > 0;
const requestId = generateId();
let method = 'GET', url = '', startTime = 0;
const reqHeaders: Record<string, string> = {};
Expand Down Expand Up @@ -1006,7 +1038,7 @@ export class DevConnect {
// Skip the start report if the fetch interceptor already
// covers this call (see handleLoadEnd for the matching skip).
const xhrKey = `${method}\0${url}`;
const isFetchRequest = (dc.fetchInFlight.get(xhrKey) ?? 0) > 0;
const isFetchRequest = isFetchXhr || (dc.fetchInFlight.get(xhrKey) ?? 0) > 0;
if (!isFetchRequest) {
dc.send('client:network:request_start', { requestId, method, url, startTime, requestHeaders: reqHeaders, requestBody, source: classifyUrl(url), via: 'xhr' });
}
Expand All @@ -1021,7 +1053,7 @@ export class DevConnect {
// XHR path) with different requestIds, which the server
// cannot merge downstream.
const xhrKey = `${method}\0${url}`;
const isFetchRequest = (dc.fetchInFlight.get(xhrKey) ?? 0) > 0;
const isFetchRequest = isFetchXhr || (dc.fetchInFlight.get(xhrKey) ?? 0) > 0;
if (isFetchRequest) {
return;
}
Expand Down
Loading