diff --git a/.changeset/metal-spies-shave.md b/.changeset/metal-spies-shave.md new file mode 100644 index 00000000000..771056629ed --- /dev/null +++ b/.changeset/metal-spies-shave.md @@ -0,0 +1,6 @@ +--- +"@firebase/database": patch +"@firebase/util": patch +--- + +Forced `get()` to wait until db is online to resolve. diff --git a/common/api-review/util.api.md b/common/api-review/util.api.md index d81fcbe8939..25657980b0e 100644 --- a/common/api-review/util.api.md +++ b/common/api-review/util.api.md @@ -346,6 +346,11 @@ export function ordinal(i: number): string; // @public (undocumented) export type PartialObserver = Partial>; +// Warning: (ae-internal-missing-underscore) The name "promiseWithTimeout" should be prefixed with an underscore because the declaration is marked as @internal +// +// @internal +export function promiseWithTimeout(promise: Promise, timeInMS?: number): Promise; + // Warning: (ae-missing-release-tag) "querystring" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) // // @public diff --git a/packages/database-compat/test/query.test.ts b/packages/database-compat/test/query.test.ts index 97faa37a427..76b57da3ff8 100644 --- a/packages/database-compat/test/query.test.ts +++ b/packages/database-compat/test/query.test.ts @@ -15,6 +15,7 @@ * limitations under the License. */ +import { promiseWithTimeout } from '@firebase/util'; import { expect, use } from 'chai'; import chaiAsPromised from 'chai-as-promised'; import * as _ from 'lodash'; @@ -3226,7 +3227,8 @@ describe('Query Tests', () => { const node = getRandomNode() as Reference; node.database.goOffline(); try { - await expect(node.get()).to.eventually.be.rejected; + const getPromise = promiseWithTimeout(node.get()); + await expect(getPromise).to.eventually.be.rejected; } finally { node.database.goOnline(); } @@ -3392,8 +3394,8 @@ describe('Query Tests', () => { expect(snapshot.val()).to.deep.equal({ data: '1' }); reader.database.goOffline(); try { - await expect(reader.child('foo/notCached').get()).to.eventually.be - .rejected; + await expect(promiseWithTimeout(reader.child('foo/notCached').get())).to + .eventually.be.rejected; } finally { reader.database.goOnline(); } diff --git a/packages/database/src/core/PersistentConnection.ts b/packages/database/src/core/PersistentConnection.ts index beeaa4e4389..4e12b37088b 100644 --- a/packages/database/src/core/PersistentConnection.ts +++ b/packages/database/src/core/PersistentConnection.ts @@ -44,7 +44,6 @@ import { QueryContext } from './view/EventRegistration'; const RECONNECT_MIN_DELAY = 1000; const RECONNECT_MAX_DELAY_DEFAULT = 60 * 5 * 1000; // 5 minutes in milliseconds (Case: 1858) -const GET_CONNECT_TIMEOUT = 3 * 1000; const RECONNECT_MAX_DELAY_FOR_ADMINS = 30 * 1000; // 30 seconds for admin clients (likely to be a backend server) const RECONNECT_DELAY_MULTIPLIER = 1.3; const RECONNECT_DELAY_RESET_TIMEOUT = 30000; // Reset delay back to MIN_DELAY after being connected for 30sec. @@ -216,22 +215,6 @@ export class PersistentConnection extends ServerActions { this.outstandingGetCount_++; const index = this.outstandingGets_.length - 1; - if (!this.connected_) { - setTimeout(() => { - const get = this.outstandingGets_[index]; - if (get === undefined || outstandingGet !== get) { - return; - } - delete this.outstandingGets_[index]; - this.outstandingGetCount_--; - if (this.outstandingGetCount_ === 0) { - this.outstandingGets_ = []; - } - this.log_('get ' + index + ' timed out on connection'); - deferred.reject(new Error('Client is offline.')); - }, GET_CONNECT_TIMEOUT); - } - if (this.connected_) { this.sendGet_(index); } diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index c695a3ee6be..3789aac7e46 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -17,7 +17,8 @@ import { initializeApp, deleteApp } from '@firebase/app'; import { Deferred } from '@firebase/util'; -import { expect } from 'chai'; +import { expect, use } from 'chai'; +import chaiAsPromised from 'chai-as-promised'; import { get, @@ -44,6 +45,8 @@ import { waitFor } from '../helpers/util'; +use(chaiAsPromised); + export function createTestApp() { return initializeApp({ databaseURL: DATABASE_URL }); } @@ -257,19 +260,6 @@ describe('Database@exp Tests', () => { expect(events[0]).to.equal('a'); }); - it('Can goOffline/goOnline', async () => { - const db = getDatabase(defaultApp); - goOffline(db); - try { - await get(ref(db, 'foo/bar')); - expect.fail('Should have failed since we are offline'); - } catch (e) { - expect(e.message).to.equal('Error: Client is offline.'); - } - goOnline(db); - await get(ref(db, 'foo/bar')); - }); - it('Can delete app', async () => { const db = getDatabase(defaultApp); await deleteApp(defaultApp); @@ -277,6 +267,31 @@ describe('Database@exp Tests', () => { defaultApp = undefined; }); + it('blocks get requests until the database is online', async () => { + const db = getDatabase(defaultApp); + const r = ref(db, 'foo3'); + const initial = { + test: 1 + }; + await set(r, initial); + goOffline(db); + const pendingGet = get(r); + let resolvedData: any = null; + pendingGet.then( + data => { + resolvedData = data; + }, + () => { + resolvedData = new Error('rejected'); + } + ); + await waitFor(2000); + expect(resolvedData).to.equal(null); + goOnline(db); + await waitFor(2000); + expect(resolvedData.val()).to.deep.equal(initial); + }); + it('Can listen to transaction changes', async () => { // Repro for https://github.com/firebase/firebase-js-sdk/issues/5195 let latestValue = 0; diff --git a/packages/util/index.node.ts b/packages/util/index.node.ts index b0021527ca0..d39d5253c1f 100644 --- a/packages/util/index.node.ts +++ b/packages/util/index.node.ts @@ -31,6 +31,7 @@ export * from './src/errors'; export * from './src/json'; export * from './src/jwt'; export * from './src/obj'; +export * from './src/promise'; export * from './src/query'; export * from './src/sha1'; export * from './src/subscribe'; diff --git a/packages/util/index.ts b/packages/util/index.ts index b4aeb5eeef3..3f38c307eb3 100644 --- a/packages/util/index.ts +++ b/packages/util/index.ts @@ -26,6 +26,7 @@ export * from './src/errors'; export * from './src/json'; export * from './src/jwt'; export * from './src/obj'; +export * from './src/promise'; export * from './src/query'; export * from './src/sha1'; export * from './src/subscribe'; diff --git a/packages/util/src/promise.ts b/packages/util/src/promise.ts new file mode 100644 index 00000000000..9bcbae1171f --- /dev/null +++ b/packages/util/src/promise.ts @@ -0,0 +1,32 @@ +/** + * @license + * Copyright 2022 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Deferred } from './deferred'; + +/** + * Rejects if the given promise doesn't resolve in timeInMS milliseconds. + * @internal + */ +export function promiseWithTimeout( + promise: Promise, + timeInMS = 2000 +): Promise { + const deferredPromise = new Deferred(); + setTimeout(() => deferredPromise.reject('timeout!'), timeInMS); + promise.then(deferredPromise.resolve, deferredPromise.reject); + return deferredPromise.promise; +}