Skip to content

Commit 926709f

Browse files
google-oss-botgoogle-oss-bot
authored
Merge main into release
2 parents 62c4c98 + 6d6ce81 commit 926709f

File tree

14 files changed

+164
-110
lines changed

14 files changed

+164
-110
lines changed

‎.changeset/itchy-boxes-try.md

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'firebase': minor
3+
'@firebase/storage': minor
4+
---
5+
6+
Migrate from the Node to Web ReadableStream interface

‎.changeset/large-games-dress.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@firebase/app': patch
3+
---
4+
5+
Prevent heartbeats methods from throwing - warn instead.

‎common/api-review/storage.api.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ export function getMetadata(ref: StorageReference): Promise<FullMetadata>;
132132
export function getStorage(app?: FirebaseApp, bucketUrl?: string): FirebaseStorage;
133133

134134
// @public
135-
export function getStream(ref: StorageReference, maxDownloadSizeBytes?: number): NodeJS.ReadableStream;
135+
export function getStream(ref: StorageReference, maxDownloadSizeBytes?: number): ReadableStream;
136136

137137
// @internal (undocumented)
138138
export function _invalidArgument(message: string): StorageError;

‎docs-devsite/storage.md

+2-2
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ This API is only available in Node.
279279
<b>Signature:</b>
280280

281281
```typescript
282-
export declare function getStream(ref: StorageReference, maxDownloadSizeBytes?: number): NodeJS.ReadableStream;
282+
export declare function getStream(ref: StorageReference, maxDownloadSizeBytes?: number): ReadableStream;
283283
```
284284

285285
#### Parameters
@@ -291,7 +291,7 @@ export declare function getStream(ref: StorageReference, maxDownloadSizeBytes?:
291291

292292
<b>Returns:</b>
293293

294-
NodeJS.ReadableStream
294+
ReadableStream
295295

296296
A stream with the object's data as bytes
297297

‎packages/app/src/heartbeatService.test.ts

+20
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,26 @@ describe('HeartbeatServiceImpl', () => {
146146
const emptyHeaders = await heartbeatService.getHeartbeatsHeader();
147147
expect(emptyHeaders).to.equal('');
148148
});
149+
it(`triggerHeartbeat() doesn't throw even if code errors`, async () => {
150+
//@ts-expect-error Ensure this doesn't match
151+
heartbeatService._heartbeatsCache?.lastSentHeartbeatDate = 50;
152+
//@ts-expect-error Ensure you can't .push() to this
153+
heartbeatService._heartbeatsCache.heartbeats = 50;
154+
const warnStub = stub(console, 'warn');
155+
await heartbeatService.triggerHeartbeat();
156+
expect(warnStub).to.be.called;
157+
expect(warnStub.args[0][1].message).to.include('heartbeats');
158+
warnStub.restore();
159+
});
160+
it(`getHeartbeatsHeader() doesn't throw even if code errors`, async () => {
161+
//@ts-expect-error Ensure you can't .push() to this
162+
heartbeatService._heartbeatsCache.heartbeats = 50;
163+
const warnStub = stub(console, 'warn');
164+
await heartbeatService.getHeartbeatsHeader();
165+
expect(warnStub).to.be.called;
166+
expect(warnStub.args[0][1].message).to.include('heartbeats');
167+
warnStub.restore();
168+
});
149169
});
150170
describe('If IndexedDB has entries', () => {
151171
let heartbeatService: HeartbeatServiceImpl;

‎packages/app/src/heartbeatService.ts

+73-63
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import {
3333
HeartbeatStorage,
3434
SingleDateHeartbeat
3535
} from './types';
36+
import { logger } from './logger';
3637

3738
const MAX_HEADER_BYTES = 1024;
3839
// 30 days
@@ -80,43 +81,47 @@ export class HeartbeatServiceImpl implements HeartbeatService {
8081
* already logged, subsequent calls to this function in the same day will be ignored.
8182
*/
8283
async triggerHeartbeat(): Promise<void> {
83-
const platformLogger = this.container
84-
.getProvider('platform-logger')
85-
.getImmediate();
84+
try {
85+
const platformLogger = this.container
86+
.getProvider('platform-logger')
87+
.getImmediate();
8688

87-
// This is the "Firebase user agent" string from the platform logger
88-
// service, not the browser user agent.
89-
const agent = platformLogger.getPlatformInfoString();
90-
const date = getUTCDateString();
91-
if (this._heartbeatsCache?.heartbeats == null) {
92-
this._heartbeatsCache = await this._heartbeatsCachePromise;
93-
// If we failed to construct a heartbeats cache, then return immediately.
89+
// This is the "Firebase user agent" string from the platform logger
90+
// service, not the browser user agent.
91+
const agent = platformLogger.getPlatformInfoString();
92+
const date = getUTCDateString();
93+
console.log('heartbeats', this._heartbeatsCache?.heartbeats);
9494
if (this._heartbeatsCache?.heartbeats == null) {
95+
this._heartbeatsCache = await this._heartbeatsCachePromise;
96+
// If we failed to construct a heartbeats cache, then return immediately.
97+
if (this._heartbeatsCache?.heartbeats == null) {
98+
return;
99+
}
100+
}
101+
// Do not store a heartbeat if one is already stored for this day
102+
// or if a header has already been sent today.
103+
if (
104+
this._heartbeatsCache.lastSentHeartbeatDate === date ||
105+
this._heartbeatsCache.heartbeats.some(
106+
singleDateHeartbeat => singleDateHeartbeat.date === date
107+
)
108+
) {
95109
return;
110+
} else {
111+
// There is no entry for this date. Create one.
112+
this._heartbeatsCache.heartbeats.push({ date, agent });
96113
}
114+
// Remove entries older than 30 days.
115+
this._heartbeatsCache.heartbeats =
116+
this._heartbeatsCache.heartbeats.filter(singleDateHeartbeat => {
117+
const hbTimestamp = new Date(singleDateHeartbeat.date).valueOf();
118+
const now = Date.now();
119+
return now - hbTimestamp <= STORED_HEARTBEAT_RETENTION_MAX_MILLIS;
120+
});
121+
return this._storage.overwrite(this._heartbeatsCache);
122+
} catch (e) {
123+
logger.warn(e);
97124
}
98-
// Do not store a heartbeat if one is already stored for this day
99-
// or if a header has already been sent today.
100-
if (
101-
this._heartbeatsCache.lastSentHeartbeatDate === date ||
102-
this._heartbeatsCache.heartbeats.some(
103-
singleDateHeartbeat => singleDateHeartbeat.date === date
104-
)
105-
) {
106-
return;
107-
} else {
108-
// There is no entry for this date. Create one.
109-
this._heartbeatsCache.heartbeats.push({ date, agent });
110-
}
111-
// Remove entries older than 30 days.
112-
this._heartbeatsCache.heartbeats = this._heartbeatsCache.heartbeats.filter(
113-
singleDateHeartbeat => {
114-
const hbTimestamp = new Date(singleDateHeartbeat.date).valueOf();
115-
const now = Date.now();
116-
return now - hbTimestamp <= STORED_HEARTBEAT_RETENTION_MAX_MILLIS;
117-
}
118-
);
119-
return this._storage.overwrite(this._heartbeatsCache);
120125
}
121126

122127
/**
@@ -127,39 +132,44 @@ export class HeartbeatServiceImpl implements HeartbeatService {
127132
* returns an empty string.
128133
*/
129134
async getHeartbeatsHeader(): Promise<string> {
130-
if (this._heartbeatsCache === null) {
131-
await this._heartbeatsCachePromise;
132-
}
133-
// If it's still null or the array is empty, there is no data to send.
134-
if (
135-
this._heartbeatsCache?.heartbeats == null ||
136-
this._heartbeatsCache.heartbeats.length === 0
137-
) {
135+
try {
136+
if (this._heartbeatsCache === null) {
137+
await this._heartbeatsCachePromise;
138+
}
139+
// If it's still null or the array is empty, there is no data to send.
140+
if (
141+
this._heartbeatsCache?.heartbeats == null ||
142+
this._heartbeatsCache.heartbeats.length === 0
143+
) {
144+
return '';
145+
}
146+
const date = getUTCDateString();
147+
// Extract as many heartbeats from the cache as will fit under the size limit.
148+
const { heartbeatsToSend, unsentEntries } = extractHeartbeatsForHeader(
149+
this._heartbeatsCache.heartbeats
150+
);
151+
const headerString = base64urlEncodeWithoutPadding(
152+
JSON.stringify({ version: 2, heartbeats: heartbeatsToSend })
153+
);
154+
// Store last sent date to prevent another being logged/sent for the same day.
155+
this._heartbeatsCache.lastSentHeartbeatDate = date;
156+
if (unsentEntries.length > 0) {
157+
// Store any unsent entries if they exist.
158+
this._heartbeatsCache.heartbeats = unsentEntries;
159+
// This seems more likely than emptying the array (below) to lead to some odd state
160+
// since the cache isn't empty and this will be called again on the next request,
161+
// and is probably safest if we await it.
162+
await this._storage.overwrite(this._heartbeatsCache);
163+
} else {
164+
this._heartbeatsCache.heartbeats = [];
165+
// Do not wait for this, to reduce latency.
166+
void this._storage.overwrite(this._heartbeatsCache);
167+
}
168+
return headerString;
169+
} catch (e) {
170+
logger.warn(e);
138171
return '';
139172
}
140-
const date = getUTCDateString();
141-
// Extract as many heartbeats from the cache as will fit under the size limit.
142-
const { heartbeatsToSend, unsentEntries } = extractHeartbeatsForHeader(
143-
this._heartbeatsCache.heartbeats
144-
);
145-
const headerString = base64urlEncodeWithoutPadding(
146-
JSON.stringify({ version: 2, heartbeats: heartbeatsToSend })
147-
);
148-
// Store last sent date to prevent another being logged/sent for the same day.
149-
this._heartbeatsCache.lastSentHeartbeatDate = date;
150-
if (unsentEntries.length > 0) {
151-
// Store any unsent entries if they exist.
152-
this._heartbeatsCache.heartbeats = unsentEntries;
153-
// This seems more likely than emptying the array (below) to lead to some odd state
154-
// since the cache isn't empty and this will be called again on the next request,
155-
// and is probably safest if we await it.
156-
await this._storage.overwrite(this._heartbeatsCache);
157-
} else {
158-
this._heartbeatsCache.heartbeats = [];
159-
// Do not wait for this, to reduce latency.
160-
void this._storage.overwrite(this._heartbeatsCache);
161-
}
162-
return headerString;
163173
}
164174
}
165175

‎packages/storage/src/api.browser.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,6 @@ export function getBlob(
5858
export function getStream(
5959
ref: StorageReference,
6060
maxDownloadSizeBytes?: number
61-
): NodeJS.ReadableStream {
61+
): ReadableStream {
6262
throw new Error('getStream() is only supported by NodeJS builds');
6363
}

‎packages/storage/src/api.node.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ export function getBlob(
5858
export function getStream(
5959
ref: StorageReference,
6060
maxDownloadSizeBytes?: number
61-
): NodeJS.ReadableStream {
61+
): ReadableStream {
6262
ref = getModularInstance(ref);
6363
return getStreamInternal(ref as Reference, maxDownloadSizeBytes);
6464
}

‎packages/storage/src/implementation/connection.ts

+1-2
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
* See the License for the specific language governing permissions and
1515
* limitations under the License.
1616
*/
17-
1817
/** Network headers */
1918
export type Headers = Record<string, string>;
2019

@@ -23,7 +22,7 @@ export type ConnectionType =
2322
| string
2423
| ArrayBuffer
2524
| Blob
26-
| NodeJS.ReadableStream;
25+
| ReadableStream<Uint8Array>;
2726

2827
/**
2928
* A lightweight wrapper around XMLHttpRequest with a

‎packages/storage/src/platform/browser/connection.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ export function newBlobConnection(): Connection<Blob> {
171171
return new XhrBlobConnection();
172172
}
173173

174-
export function newStreamConnection(): Connection<NodeJS.ReadableStream> {
174+
export function newStreamConnection(): Connection<ReadableStream> {
175175
throw new Error('Streams are only supported on Node');
176176
}
177177

‎packages/storage/src/platform/connection.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ export function newBlobConnection(): Connection<Blob> {
4545
return nodeNewBlobConnection();
4646
}
4747

48-
export function newStreamConnection(): Connection<NodeJS.ReadableStream> {
48+
export function newStreamConnection(): Connection<ReadableStream<Uint8Array>> {
4949
// This file is only used in Node.js tests using ts-node.
5050
return nodeNewStreamConnection();
5151
}

‎packages/storage/src/platform/node/connection.ts

+11-9
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ abstract class FetchConnection<T extends ConnectionType>
5050
async send(
5151
url: string,
5252
method: string,
53-
body?: ArrayBufferView | Blob | string,
53+
body?: NodeJS.ArrayBufferView | Blob | string,
5454
headers?: Record<string, string>
5555
): Promise<void> {
5656
if (this.sent_) {
@@ -62,7 +62,7 @@ abstract class FetchConnection<T extends ConnectionType>
6262
const response = await this.fetch_(url, {
6363
method,
6464
headers: headers || {},
65-
body: body as ArrayBufferView | string
65+
body: body as NodeJS.ArrayBufferView | string
6666
});
6767
this.headers_ = response.headers;
6868
this.statusCode_ = response.status;
@@ -146,13 +146,15 @@ export function newBytesConnection(): Connection<ArrayBuffer> {
146146
return new FetchBytesConnection();
147147
}
148148

149-
export class FetchStreamConnection extends FetchConnection<NodeJS.ReadableStream> {
150-
private stream_: NodeJS.ReadableStream | null = null;
149+
export class FetchStreamConnection extends FetchConnection<
150+
ReadableStream<Uint8Array>
151+
> {
152+
private stream_: ReadableStream<Uint8Array> | null = null;
151153

152154
async send(
153155
url: string,
154156
method: string,
155-
body?: ArrayBufferView | Blob | string,
157+
body?: NodeJS.ArrayBufferView | Blob | string,
156158
headers?: Record<string, string>
157159
): Promise<void> {
158160
if (this.sent_) {
@@ -164,12 +166,12 @@ export class FetchStreamConnection extends FetchConnection<NodeJS.ReadableStream
164166
const response = await this.fetch_(url, {
165167
method,
166168
headers: headers || {},
167-
body: body as ArrayBufferView | string
169+
body: body as NodeJS.ArrayBufferView | string
168170
});
169171
this.headers_ = response.headers;
170172
this.statusCode_ = response.status;
171173
this.errorCode_ = ErrorCode.NO_ERROR;
172-
this.stream_ = response.body;
174+
this.stream_ = response.body as ReadableStream<Uint8Array>;
173175
} catch (e) {
174176
this.errorText_ = (e as Error)?.message;
175177
// emulate XHR which sets status to 0 when encountering a network error
@@ -178,15 +180,15 @@ export class FetchStreamConnection extends FetchConnection<NodeJS.ReadableStream
178180
}
179181
}
180182

181-
getResponse(): NodeJS.ReadableStream {
183+
getResponse(): ReadableStream {
182184
if (!this.stream_) {
183185
throw internalError('cannot .getResponse() before sending');
184186
}
185187
return this.stream_;
186188
}
187189
}
188190

189-
export function newStreamConnection(): Connection<NodeJS.ReadableStream> {
191+
export function newStreamConnection(): Connection<ReadableStream<Uint8Array>> {
190192
return new FetchStreamConnection();
191193
}
192194

0 commit comments

Comments
 (0)