From 6d6ce8100fd1443a40cd1ebd68ad980cab55fb10 Mon Sep 17 00:00:00 2001 From: Christina Holland Date: Wed, 14 Aug 2024 11:25:05 -0700 Subject: [PATCH] Wrap all heartbeats methods in try/catch (#8425) --- .changeset/large-games-dress.md | 5 + packages/app/src/heartbeatService.test.ts | 20 ++++ packages/app/src/heartbeatService.ts | 136 ++++++++++++---------- 3 files changed, 98 insertions(+), 63 deletions(-) create mode 100644 .changeset/large-games-dress.md diff --git a/.changeset/large-games-dress.md b/.changeset/large-games-dress.md new file mode 100644 index 00000000000..126ef4049ab --- /dev/null +++ b/.changeset/large-games-dress.md @@ -0,0 +1,5 @@ +--- +'@firebase/app': patch +--- + +Prevent heartbeats methods from throwing - warn instead. diff --git a/packages/app/src/heartbeatService.test.ts b/packages/app/src/heartbeatService.test.ts index 8877cb342fc..95ac71ca42d 100644 --- a/packages/app/src/heartbeatService.test.ts +++ b/packages/app/src/heartbeatService.test.ts @@ -146,6 +146,26 @@ describe('HeartbeatServiceImpl', () => { const emptyHeaders = await heartbeatService.getHeartbeatsHeader(); expect(emptyHeaders).to.equal(''); }); + it(`triggerHeartbeat() doesn't throw even if code errors`, async () => { + //@ts-expect-error Ensure this doesn't match + heartbeatService._heartbeatsCache?.lastSentHeartbeatDate = 50; + //@ts-expect-error Ensure you can't .push() to this + heartbeatService._heartbeatsCache.heartbeats = 50; + const warnStub = stub(console, 'warn'); + await heartbeatService.triggerHeartbeat(); + expect(warnStub).to.be.called; + expect(warnStub.args[0][1].message).to.include('heartbeats'); + warnStub.restore(); + }); + it(`getHeartbeatsHeader() doesn't throw even if code errors`, async () => { + //@ts-expect-error Ensure you can't .push() to this + heartbeatService._heartbeatsCache.heartbeats = 50; + const warnStub = stub(console, 'warn'); + await heartbeatService.getHeartbeatsHeader(); + expect(warnStub).to.be.called; + expect(warnStub.args[0][1].message).to.include('heartbeats'); + warnStub.restore(); + }); }); describe('If IndexedDB has entries', () => { let heartbeatService: HeartbeatServiceImpl; diff --git a/packages/app/src/heartbeatService.ts b/packages/app/src/heartbeatService.ts index fbe24242654..6bf9464319b 100644 --- a/packages/app/src/heartbeatService.ts +++ b/packages/app/src/heartbeatService.ts @@ -33,6 +33,7 @@ import { HeartbeatStorage, SingleDateHeartbeat } from './types'; +import { logger } from './logger'; const MAX_HEADER_BYTES = 1024; // 30 days @@ -80,43 +81,47 @@ export class HeartbeatServiceImpl implements HeartbeatService { * already logged, subsequent calls to this function in the same day will be ignored. */ async triggerHeartbeat(): Promise { - const platformLogger = this.container - .getProvider('platform-logger') - .getImmediate(); + try { + const platformLogger = this.container + .getProvider('platform-logger') + .getImmediate(); - // This is the "Firebase user agent" string from the platform logger - // service, not the browser user agent. - const agent = platformLogger.getPlatformInfoString(); - const date = getUTCDateString(); - if (this._heartbeatsCache?.heartbeats == null) { - this._heartbeatsCache = await this._heartbeatsCachePromise; - // If we failed to construct a heartbeats cache, then return immediately. + // This is the "Firebase user agent" string from the platform logger + // service, not the browser user agent. + const agent = platformLogger.getPlatformInfoString(); + const date = getUTCDateString(); + console.log('heartbeats', this._heartbeatsCache?.heartbeats); if (this._heartbeatsCache?.heartbeats == null) { + this._heartbeatsCache = await this._heartbeatsCachePromise; + // If we failed to construct a heartbeats cache, then return immediately. + if (this._heartbeatsCache?.heartbeats == null) { + return; + } + } + // Do not store a heartbeat if one is already stored for this day + // or if a header has already been sent today. + if ( + this._heartbeatsCache.lastSentHeartbeatDate === date || + this._heartbeatsCache.heartbeats.some( + singleDateHeartbeat => singleDateHeartbeat.date === date + ) + ) { return; + } else { + // There is no entry for this date. Create one. + this._heartbeatsCache.heartbeats.push({ date, agent }); } + // Remove entries older than 30 days. + this._heartbeatsCache.heartbeats = + this._heartbeatsCache.heartbeats.filter(singleDateHeartbeat => { + const hbTimestamp = new Date(singleDateHeartbeat.date).valueOf(); + const now = Date.now(); + return now - hbTimestamp <= STORED_HEARTBEAT_RETENTION_MAX_MILLIS; + }); + return this._storage.overwrite(this._heartbeatsCache); + } catch (e) { + logger.warn(e); } - // Do not store a heartbeat if one is already stored for this day - // or if a header has already been sent today. - if ( - this._heartbeatsCache.lastSentHeartbeatDate === date || - this._heartbeatsCache.heartbeats.some( - singleDateHeartbeat => singleDateHeartbeat.date === date - ) - ) { - return; - } else { - // There is no entry for this date. Create one. - this._heartbeatsCache.heartbeats.push({ date, agent }); - } - // Remove entries older than 30 days. - this._heartbeatsCache.heartbeats = this._heartbeatsCache.heartbeats.filter( - singleDateHeartbeat => { - const hbTimestamp = new Date(singleDateHeartbeat.date).valueOf(); - const now = Date.now(); - return now - hbTimestamp <= STORED_HEARTBEAT_RETENTION_MAX_MILLIS; - } - ); - return this._storage.overwrite(this._heartbeatsCache); } /** @@ -127,39 +132,44 @@ export class HeartbeatServiceImpl implements HeartbeatService { * returns an empty string. */ async getHeartbeatsHeader(): Promise { - if (this._heartbeatsCache === null) { - await this._heartbeatsCachePromise; - } - // If it's still null or the array is empty, there is no data to send. - if ( - this._heartbeatsCache?.heartbeats == null || - this._heartbeatsCache.heartbeats.length === 0 - ) { + try { + if (this._heartbeatsCache === null) { + await this._heartbeatsCachePromise; + } + // If it's still null or the array is empty, there is no data to send. + if ( + this._heartbeatsCache?.heartbeats == null || + this._heartbeatsCache.heartbeats.length === 0 + ) { + return ''; + } + const date = getUTCDateString(); + // Extract as many heartbeats from the cache as will fit under the size limit. + const { heartbeatsToSend, unsentEntries } = extractHeartbeatsForHeader( + this._heartbeatsCache.heartbeats + ); + const headerString = base64urlEncodeWithoutPadding( + JSON.stringify({ version: 2, heartbeats: heartbeatsToSend }) + ); + // Store last sent date to prevent another being logged/sent for the same day. + this._heartbeatsCache.lastSentHeartbeatDate = date; + if (unsentEntries.length > 0) { + // Store any unsent entries if they exist. + this._heartbeatsCache.heartbeats = unsentEntries; + // This seems more likely than emptying the array (below) to lead to some odd state + // since the cache isn't empty and this will be called again on the next request, + // and is probably safest if we await it. + await this._storage.overwrite(this._heartbeatsCache); + } else { + this._heartbeatsCache.heartbeats = []; + // Do not wait for this, to reduce latency. + void this._storage.overwrite(this._heartbeatsCache); + } + return headerString; + } catch (e) { + logger.warn(e); return ''; } - const date = getUTCDateString(); - // Extract as many heartbeats from the cache as will fit under the size limit. - const { heartbeatsToSend, unsentEntries } = extractHeartbeatsForHeader( - this._heartbeatsCache.heartbeats - ); - const headerString = base64urlEncodeWithoutPadding( - JSON.stringify({ version: 2, heartbeats: heartbeatsToSend }) - ); - // Store last sent date to prevent another being logged/sent for the same day. - this._heartbeatsCache.lastSentHeartbeatDate = date; - if (unsentEntries.length > 0) { - // Store any unsent entries if they exist. - this._heartbeatsCache.heartbeats = unsentEntries; - // This seems more likely than emptying the array (below) to lead to some odd state - // since the cache isn't empty and this will be called again on the next request, - // and is probably safest if we await it. - await this._storage.overwrite(this._heartbeatsCache); - } else { - this._heartbeatsCache.heartbeats = []; - // Do not wait for this, to reduce latency. - void this._storage.overwrite(this._heartbeatsCache); - } - return headerString; } }