Skip to content

Commit ca14537

Browse files
committed
Clear NetworkReporter timing data for failed requests
1 parent 5a29c68 commit ca14537

3 files changed

Lines changed: 41 additions & 2 deletions

File tree

packages/react-native/ReactCommon/jsinspector-modern/tests/NetworkReporterTest.cpp

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include <oscompat/OSCompat.h>
1414
#include <react/featureflags/ReactNativeFeatureFlags.h>
1515
#include <react/networking/NetworkReporter.h>
16+
#include <react/performance/timeline/PerformanceEntryReporter.h>
1617

1718
using namespace ::testing;
1819

@@ -282,6 +283,38 @@ TEST_P(NetworkReporterTest, testLoadingFailedError) {
282283
})");
283284
}
284285

286+
TEST_P(NetworkReporterTest, testFailedRequestDiscardsResourceTimingData) {
287+
auto& networkReporter = NetworkReporter::getInstance();
288+
auto performanceEntryReporter = PerformanceEntryReporter::getInstance();
289+
const std::string requestId = "failed-request-resource-timing";
290+
const std::string retryUrl = "https://example.com/retry";
291+
292+
performanceEntryReporter->clearEntries(PerformanceEntryType::RESOURCE);
293+
294+
networkReporter.reportRequestStart(
295+
requestId,
296+
{.url = "https://example.com/failed", .httpMethod = "GET"},
297+
0,
298+
std::nullopt);
299+
networkReporter.reportRequestFailed(requestId, false);
300+
301+
// Reusing the ID makes retained timing data observable without exposing
302+
// NetworkReporter internals.
303+
networkReporter.reportRequestStart(
304+
requestId, {.url = retryUrl, .httpMethod = "GET"}, 0, std::nullopt);
305+
networkReporter.reportResponseStart(
306+
requestId, {.url = retryUrl, .statusCode = 200}, 0);
307+
networkReporter.reportResponseEnd(requestId, 0);
308+
309+
const auto entries =
310+
performanceEntryReporter->getEntries(PerformanceEntryType::RESOURCE);
311+
ASSERT_EQ(1, entries.size());
312+
EXPECT_EQ(
313+
retryUrl, std::get<PerformanceResourceTiming>(entries.front()).name);
314+
315+
performanceEntryReporter->clearEntries(PerformanceEntryType::RESOURCE);
316+
}
317+
285318
TEST_P(NetworkReporterTest, testCompleteNetworkFlow) {
286319
InSequence s;
287320
this->expectMessageFromPage(JsonEq(R"({

packages/react-native/ReactCommon/react/networking/NetworkReporter.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,12 @@ void NetworkReporter::reportResponseEnd(
234234
void NetworkReporter::reportRequestFailed(
235235
const std::string& requestId,
236236
bool cancelled) const {
237+
// All builds: Discard incomplete PerformanceResourceTiming metadata
238+
{
239+
std::lock_guard<std::mutex> lock(perfTimingsMutex_);
240+
perfTimingsBuffer_.erase(requestId);
241+
}
242+
237243
#ifdef REACT_NATIVE_DEBUGGER_ENABLED
238244
// Debugger enabled: CDP event handling
239245
jsinspector_modern::NetworkHandler::getInstance().onLoadingFailed(

packages/react-native/ReactCommon/react/networking/NetworkReporter.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,8 +200,8 @@ class NetworkReporter {
200200
NetworkReporter &operator=(const NetworkReporter &) = delete;
201201
~NetworkReporter() = default;
202202

203-
std::unordered_map<std::string, ResourceTimingData> perfTimingsBuffer_{};
204-
std::mutex perfTimingsMutex_;
203+
mutable std::unordered_map<std::string, ResourceTimingData> perfTimingsBuffer_{};
204+
mutable std::mutex perfTimingsMutex_;
205205
};
206206

207207
} // namespace facebook::react

0 commit comments

Comments
 (0)