From 42fb9776b3fecd3af5a01251737eaf9ec3539061 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Tue, 7 Jun 2022 16:55:54 -0700 Subject: [PATCH 01/16] Forced get to wait until db is online --- .../database/src/core/PersistentConnection.ts | 17 ------------ .../database/test/exp/integration.test.ts | 26 +++++++++++++++++++ 2 files changed, 26 insertions(+), 17 deletions(-) diff --git a/packages/database/src/core/PersistentConnection.ts b/packages/database/src/core/PersistentConnection.ts index 62b27a37686..888c9b9d4ce 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. @@ -222,22 +221,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 0d0858c28c8..e2707403e8c 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -154,6 +154,32 @@ describe('Database@exp Tests', () => { defaultApp = undefined; }); + it.only('waits until the database is online to resolve the get request', async () => { + const db = getDatabase(defaultApp); + goOffline(db); + const r = ref(db, 'foo2'); + let resolved = false; + const getValue = get(r); + getValue.then( + () => (resolved = true), + () => (resolved = true) + ); + // TODO: use new API + const deferredTimeout = new Deferred(); + setTimeout(() => { + deferredTimeout.resolve(); + }, 2000); + await deferredTimeout.promise; + expect(resolved).to.equal(false); + goOnline(db); + const deferredTimeout2 = new Deferred(); + setTimeout(() => { + deferredTimeout2.resolve(); + }, 2000); + await deferredTimeout2.promise; + expect(resolved).to.equal(true); + }); + it('Can listen to transaction changes', async () => { // Repro for https://github.com/firebase/firebase-js-sdk/issues/5195 let latestValue = 0; From b70b40514f19677c563c9493f4b3d21cd1bdecfe Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Tue, 7 Jun 2022 16:56:39 -0700 Subject: [PATCH 02/16] Create metal-spies-shave.md --- .changeset/metal-spies-shave.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/metal-spies-shave.md diff --git a/.changeset/metal-spies-shave.md b/.changeset/metal-spies-shave.md new file mode 100644 index 00000000000..9484923e39a --- /dev/null +++ b/.changeset/metal-spies-shave.md @@ -0,0 +1,5 @@ +--- +"@firebase/database": patch +--- + +Forced get to wait until db is online From 6f25a80c343360c89653e9a1cc1e8daa38f92239 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Wed, 15 Jun 2022 09:00:22 -0700 Subject: [PATCH 03/16] Removed erroneous test --- packages/database/test/exp/integration.test.ts | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index e2707403e8c..1c52573e8ad 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -134,19 +134,6 @@ describe('Database@exp Tests', () => { expect(snap1).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); @@ -154,7 +141,7 @@ describe('Database@exp Tests', () => { defaultApp = undefined; }); - it.only('waits until the database is online to resolve the get request', async () => { + it('waits until the database is online to resolve the get request', async () => { const db = getDatabase(defaultApp); goOffline(db); const r = ref(db, 'foo2'); From 2ca5bae0600761e324a64766bae556cfbfb8dcc7 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Wed, 15 Jun 2022 17:17:59 -0700 Subject: [PATCH 04/16] WIP --- packages/database-compat/test/query.test.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/database-compat/test/query.test.ts b/packages/database-compat/test/query.test.ts index 97faa37a427..3874eb3eb95 100644 --- a/packages/database-compat/test/query.test.ts +++ b/packages/database-compat/test/query.test.ts @@ -3222,11 +3222,15 @@ describe('Query Tests', () => { expect(snapshot.val()).to.be.null; }); - it('get for missing node while offline is rejected', async () => { + it.only('get for missing node while offline is rejected', async () => { const node = getRandomNode() as Reference; node.database.goOffline(); try { - await expect(node.get()).to.eventually.be.rejected; + const getPromise = new Promise((resolve, reject) => { + setTimeout(reject, 2000); + node.get().then(resolve); + }); + await expect(getPromise).to.eventually.be.rejected; } finally { node.database.goOnline(); } From 96ca94a2ec41225ec7644a0e6e7522ced1d8c88a Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Thu, 16 Jun 2022 15:58:32 -0700 Subject: [PATCH 05/16] Fixed some tests --- packages/database-compat/test/query.test.ts | 2 +- .../database/test/exp/integration.test.ts | 38 ++++++++----------- packages/database/test/helpers/util.ts | 10 +++++ 3 files changed, 26 insertions(+), 24 deletions(-) diff --git a/packages/database-compat/test/query.test.ts b/packages/database-compat/test/query.test.ts index 3874eb3eb95..a5cbd87b55b 100644 --- a/packages/database-compat/test/query.test.ts +++ b/packages/database-compat/test/query.test.ts @@ -3222,7 +3222,7 @@ describe('Query Tests', () => { expect(snapshot.val()).to.be.null; }); - it.only('get for missing node while offline is rejected', async () => { + it('get for missing node while offline is rejected', async () => { const node = getRandomNode() as Reference; node.database.goOffline(); try { diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index 1c52573e8ad..64fa947a43c 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -18,7 +18,8 @@ // eslint-disable-next-line import/no-extraneous-dependencies 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 { onValue, set } from '../../src/api/Reference_impl'; import { @@ -32,7 +33,9 @@ import { runTransaction } from '../../src/index'; import { EventAccumulatorFactory } from '../helpers/EventAccumulator'; -import { DATABASE_ADDRESS, DATABASE_URL } from '../helpers/util'; +import { DATABASE_ADDRESS, DATABASE_URL, timeoutResolve } from '../helpers/util'; + +use(chaiAsPromised); export function createTestApp() { return initializeApp({ databaseURL: DATABASE_URL }); @@ -140,31 +143,20 @@ describe('Database@exp Tests', () => { expect(() => ref(db)).to.throw('Cannot call ref on a deleted database.'); defaultApp = undefined; }); - + it('waits until the database is online to resolve the get request', async () => { const db = getDatabase(defaultApp); - goOffline(db); const r = ref(db, 'foo2'); - let resolved = false; - const getValue = get(r); - getValue.then( - () => (resolved = true), - () => (resolved = true) - ); - // TODO: use new API - const deferredTimeout = new Deferred(); - setTimeout(() => { - deferredTimeout.resolve(); - }, 2000); - await deferredTimeout.promise; - expect(resolved).to.equal(false); + const initial = { + test: 1 + }; + await set(r, initial); + goOffline(db); + const getValue = timeoutResolve(get(r)); + expect(getValue).to.be.rejectedWith('timeout!'); goOnline(db); - const deferredTimeout2 = new Deferred(); - setTimeout(() => { - deferredTimeout2.resolve(); - }, 2000); - await deferredTimeout2.promise; - expect(resolved).to.equal(true); + const deferredTimeout2 = await timeoutResolve(get(r)); + expect(deferredTimeout2.val()).to.deep.equal(initial); }); it('Can listen to transaction changes', async () => { diff --git a/packages/database/test/helpers/util.ts b/packages/database/test/helpers/util.ts index d50d42bd311..72b7e1fdb77 100644 --- a/packages/database/test/helpers/util.ts +++ b/packages/database/test/helpers/util.ts @@ -15,6 +15,8 @@ * limitations under the License. */ +import { Deferred } from '@firebase/util'; + import { ConnectionTarget } from '../../src/api/test_access'; // eslint-disable-next-line @typescript-eslint/no-require-imports @@ -70,3 +72,11 @@ export function shuffle(arr, randFn = Math.random) { arr[j] = tmp; } } + +export function timeoutResolve(promise: Promise, timeInMS = 2000) { + const deferredPromise = new Deferred(); + setTimeout(() => deferredPromise.reject('timeout!'), timeInMS); + promise.then(deferredPromise.resolve, deferredPromise.reject); + return deferredPromise.promise; +} + From b4771e2006357e2383fbe54823d6642a05ef7794 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Thu, 16 Jun 2022 16:22:25 -0700 Subject: [PATCH 06/16] Used utility function --- packages/database-compat/test/helpers/util.ts | 9 +++++++++ packages/database-compat/test/query.test.ts | 7 ++----- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/packages/database-compat/test/helpers/util.ts b/packages/database-compat/test/helpers/util.ts index a932c929843..4d92bd2571f 100644 --- a/packages/database-compat/test/helpers/util.ts +++ b/packages/database-compat/test/helpers/util.ts @@ -22,6 +22,7 @@ import '../../src/index'; import firebase from '@firebase/app-compat'; import { _FirebaseNamespace } from '@firebase/app-types/private'; import { Component, ComponentType } from '@firebase/component'; +import { Deferred } from '@firebase/util'; import { Path } from '../../../database/src/core/util/Path'; import { Query, Reference } from '../../src/api/Reference'; @@ -175,3 +176,11 @@ export function canCreateExtraConnections() { typeof MozWebSocket !== 'undefined' || typeof WebSocket !== 'undefined' ); } + +// TODO: Move this to @firebase/util +export function timeoutResolve(promise: Promise, timeInMS = 2000) { + const deferredPromise = new Deferred(); + setTimeout(() => deferredPromise.reject('timeout!'), timeInMS); + promise.then(deferredPromise.resolve, deferredPromise.reject); + return deferredPromise.promise; +} diff --git a/packages/database-compat/test/query.test.ts b/packages/database-compat/test/query.test.ts index a5cbd87b55b..7da3f664263 100644 --- a/packages/database-compat/test/query.test.ts +++ b/packages/database-compat/test/query.test.ts @@ -29,7 +29,7 @@ import { } from '../../database/test/helpers/EventAccumulator'; import { DataSnapshot, Query, Reference } from '../src/api/Reference'; -import { getFreshRepo, getPath, getRandomNode, pause } from './helpers/util'; +import { getFreshRepo, getPath, getRandomNode, pause, timeoutResolve } from './helpers/util'; use(chaiAsPromised); @@ -3226,10 +3226,7 @@ describe('Query Tests', () => { const node = getRandomNode() as Reference; node.database.goOffline(); try { - const getPromise = new Promise((resolve, reject) => { - setTimeout(reject, 2000); - node.get().then(resolve); - }); + const getPromise = timeoutResolve(node.get()); await expect(getPromise).to.eventually.be.rejected; } finally { node.database.goOnline(); From 2d553429d8637ef3996b566de1c883c6a4b4604c Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Thu, 16 Jun 2022 16:37:34 -0700 Subject: [PATCH 07/16] Added util function --- common/api-review/util.api.md | 5 ++++ packages/database-compat/test/query.test.ts | 3 +- .../database/test/exp/integration.test.ts | 4 +-- packages/database/test/helpers/util.ts | 10 ------- packages/util/index.node.ts | 1 + packages/util/index.ts | 1 + packages/util/src/promise.ts | 29 +++++++++++++++++++ 7 files changed, 40 insertions(+), 13 deletions(-) create mode 100644 packages/util/src/promise.ts diff --git a/common/api-review/util.api.md b/common/api-review/util.api.md index 4c1f055e022..d4f1c7ae15c 100644 --- a/common/api-review/util.api.md +++ b/common/api-review/util.api.md @@ -417,6 +417,11 @@ export interface Subscribe { (observer: PartialObserver): Unsubscribe; } +// Warning: (ae-missing-release-tag) "timeoutResolve" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) +// +// @public +export function timeoutResolve(promise: Promise, timeInMS?: number): Promise; + // Warning: (ae-missing-release-tag) "Unsubscribe" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) // // @public (undocumented) diff --git a/packages/database-compat/test/query.test.ts b/packages/database-compat/test/query.test.ts index 7da3f664263..980a7607cea 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 { timeoutResolve } from '@firebase/util'; import { expect, use } from 'chai'; import chaiAsPromised from 'chai-as-promised'; import * as _ from 'lodash'; @@ -29,7 +30,7 @@ import { } from '../../database/test/helpers/EventAccumulator'; import { DataSnapshot, Query, Reference } from '../src/api/Reference'; -import { getFreshRepo, getPath, getRandomNode, pause, timeoutResolve } from './helpers/util'; +import { getFreshRepo, getPath, getRandomNode, pause } from './helpers/util'; use(chaiAsPromised); diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index 64fa947a43c..26cf23d11d7 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -17,7 +17,7 @@ // eslint-disable-next-line import/no-extraneous-dependencies import { initializeApp, deleteApp } from '@firebase/app'; -import { Deferred } from '@firebase/util'; +import { Deferred, timeoutResolve } from '@firebase/util'; import { expect, use } from 'chai'; import chaiAsPromised from 'chai-as-promised'; @@ -33,7 +33,7 @@ import { runTransaction } from '../../src/index'; import { EventAccumulatorFactory } from '../helpers/EventAccumulator'; -import { DATABASE_ADDRESS, DATABASE_URL, timeoutResolve } from '../helpers/util'; +import { DATABASE_ADDRESS, DATABASE_URL } from '../helpers/util'; use(chaiAsPromised); diff --git a/packages/database/test/helpers/util.ts b/packages/database/test/helpers/util.ts index 72b7e1fdb77..d50d42bd311 100644 --- a/packages/database/test/helpers/util.ts +++ b/packages/database/test/helpers/util.ts @@ -15,8 +15,6 @@ * limitations under the License. */ -import { Deferred } from '@firebase/util'; - import { ConnectionTarget } from '../../src/api/test_access'; // eslint-disable-next-line @typescript-eslint/no-require-imports @@ -72,11 +70,3 @@ export function shuffle(arr, randFn = Math.random) { arr[j] = tmp; } } - -export function timeoutResolve(promise: Promise, timeInMS = 2000) { - const deferredPromise = new Deferred(); - setTimeout(() => deferredPromise.reject('timeout!'), timeInMS); - promise.then(deferredPromise.resolve, deferredPromise.reject); - return deferredPromise.promise; -} - diff --git a/packages/util/index.node.ts b/packages/util/index.node.ts index 8dace3b8e1e..c8f3b2f2ede 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 00d661734b8..b6a2ac6267b 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..d45fb18d2c0 --- /dev/null +++ b/packages/util/src/promise.ts @@ -0,0 +1,29 @@ +/** + * @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 timeoutResolve(promise: Promise, timeInMS = 2000) { + const deferredPromise = new Deferred(); + setTimeout(() => deferredPromise.reject('timeout!'), timeInMS); + promise.then(deferredPromise.resolve, deferredPromise.reject); + return deferredPromise.promise; +} From dd0a25afe4eb1f0a56133cd95a5dcf0f528535b5 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Thu, 16 Jun 2022 16:39:50 -0700 Subject: [PATCH 08/16] Removed extraneous file from util --- packages/database-compat/test/helpers/util.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/packages/database-compat/test/helpers/util.ts b/packages/database-compat/test/helpers/util.ts index 4d92bd2571f..402a1bd7643 100644 --- a/packages/database-compat/test/helpers/util.ts +++ b/packages/database-compat/test/helpers/util.ts @@ -176,11 +176,3 @@ export function canCreateExtraConnections() { typeof MozWebSocket !== 'undefined' || typeof WebSocket !== 'undefined' ); } - -// TODO: Move this to @firebase/util -export function timeoutResolve(promise: Promise, timeInMS = 2000) { - const deferredPromise = new Deferred(); - setTimeout(() => deferredPromise.reject('timeout!'), timeInMS); - promise.then(deferredPromise.resolve, deferredPromise.reject); - return deferredPromise.promise; -} From ffd922249d85ff7c3ccfd32808e4772aa658df5d Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Thu, 16 Jun 2022 19:32:03 -0700 Subject: [PATCH 09/16] Removed unnecessary import --- packages/database-compat/test/helpers/util.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/database-compat/test/helpers/util.ts b/packages/database-compat/test/helpers/util.ts index 402a1bd7643..a932c929843 100644 --- a/packages/database-compat/test/helpers/util.ts +++ b/packages/database-compat/test/helpers/util.ts @@ -22,7 +22,6 @@ import '../../src/index'; import firebase from '@firebase/app-compat'; import { _FirebaseNamespace } from '@firebase/app-types/private'; import { Component, ComponentType } from '@firebase/component'; -import { Deferred } from '@firebase/util'; import { Path } from '../../../database/src/core/util/Path'; import { Query, Reference } from '../../src/api/Reference'; From 96ffdd129830a8fd9dec9d0f6931db10ea83ab29 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Thu, 16 Jun 2022 19:39:35 -0700 Subject: [PATCH 10/16] Renamed function --- common/api-review/util.api.md | 10 +++++----- packages/database-compat/test/query.test.ts | 4 ++-- packages/database/test/exp/integration.test.ts | 6 +++--- packages/util/src/promise.ts | 10 +++++----- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/common/api-review/util.api.md b/common/api-review/util.api.md index 7886458fd42..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 @@ -417,11 +422,6 @@ export interface Subscribe { (observer: PartialObserver): Unsubscribe; } -// Warning: (ae-missing-release-tag) "timeoutResolve" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) -// -// @public -export function timeoutResolve(promise: Promise, timeInMS?: number): Promise; - // Warning: (ae-missing-release-tag) "Unsubscribe" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) // // @public (undocumented) diff --git a/packages/database-compat/test/query.test.ts b/packages/database-compat/test/query.test.ts index 980a7607cea..ac58efa97e7 100644 --- a/packages/database-compat/test/query.test.ts +++ b/packages/database-compat/test/query.test.ts @@ -15,7 +15,7 @@ * limitations under the License. */ -import { timeoutResolve } from '@firebase/util'; +import { promiseWithTimeout } from '@firebase/util'; import { expect, use } from 'chai'; import chaiAsPromised from 'chai-as-promised'; import * as _ from 'lodash'; @@ -3227,7 +3227,7 @@ describe('Query Tests', () => { const node = getRandomNode() as Reference; node.database.goOffline(); try { - const getPromise = timeoutResolve(node.get()); + const getPromise = promiseWithTimeout(node.get()); await expect(getPromise).to.eventually.be.rejected; } finally { node.database.goOnline(); diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index 7aa0c69db77..df74987beab 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -16,7 +16,7 @@ */ import { initializeApp, deleteApp } from '@firebase/app'; -import { Deferred, timeoutResolve } from '@firebase/util'; +import { Deferred, promiseWithTimeout } from '@firebase/util'; import { expect, use } from 'chai'; import chaiAsPromised from 'chai-as-promised'; @@ -275,10 +275,10 @@ describe('Database@exp Tests', () => { }; await set(r, initial); goOffline(db); - const getValue = timeoutResolve(get(r)); + const getValue = promiseWithTimeout(get(r)); expect(getValue).to.be.rejectedWith('timeout!'); goOnline(db); - const deferredTimeout2 = await timeoutResolve(get(r)); + const deferredTimeout2 = await promiseWithTimeout(get(r)); expect(deferredTimeout2.val()).to.deep.equal(initial); }); diff --git a/packages/util/src/promise.ts b/packages/util/src/promise.ts index d45fb18d2c0..0592cbbe825 100644 --- a/packages/util/src/promise.ts +++ b/packages/util/src/promise.ts @@ -17,11 +17,11 @@ import { Deferred } from "./deferred"; -/* - Rejects if the given promise doesn't resolve in timeInMS milliseconds. - @internal -*/ -export function timeoutResolve(promise: Promise, timeInMS = 2000) { +/** + * 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); From 0a0fb3a6f58500c90ea7a412b58551e2a567dcc1 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Thu, 16 Jun 2022 19:39:52 -0700 Subject: [PATCH 11/16] Updated formatting --- packages/database/test/exp/integration.test.ts | 2 +- packages/util/src/promise.ts | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index df74987beab..50533879c84 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -266,7 +266,7 @@ describe('Database@exp Tests', () => { expect(() => ref(db)).to.throw('Cannot call ref on a deleted database.'); defaultApp = undefined; }); - + it('waits until the database is online to resolve the get request', async () => { const db = getDatabase(defaultApp); const r = ref(db, 'foo2'); diff --git a/packages/util/src/promise.ts b/packages/util/src/promise.ts index 0592cbbe825..9bcbae1171f 100644 --- a/packages/util/src/promise.ts +++ b/packages/util/src/promise.ts @@ -15,13 +15,16 @@ * limitations under the License. */ -import { Deferred } from "./deferred"; +import { Deferred } from './deferred'; /** * Rejects if the given promise doesn't resolve in timeInMS milliseconds. * @internal */ -export function promiseWithTimeout(promise: Promise, timeInMS = 2000): Promise { +export function promiseWithTimeout( + promise: Promise, + timeInMS = 2000 +): Promise { const deferredPromise = new Deferred(); setTimeout(() => deferredPromise.reject('timeout!'), timeInMS); promise.then(deferredPromise.resolve, deferredPromise.reject); From 5e8ead831420ff108d9a65d8bccc32f15a252e1d Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Thu, 16 Jun 2022 20:20:29 -0700 Subject: [PATCH 12/16] Attempted to fix timeout --- packages/database-compat/test/query.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/database-compat/test/query.test.ts b/packages/database-compat/test/query.test.ts index ac58efa97e7..4cfe7416616 100644 --- a/packages/database-compat/test/query.test.ts +++ b/packages/database-compat/test/query.test.ts @@ -3394,7 +3394,7 @@ 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 + await expect(promiseWithTimeout(reader.child('foo/notCached').get())).to.eventually.be .rejected; } finally { reader.database.goOnline(); From c8bf25ecd2dd41e2f1ac9573f07f7a0641e6451d Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Thu, 16 Jun 2022 21:06:55 -0700 Subject: [PATCH 13/16] Fixed formatting --- packages/database-compat/test/query.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/database-compat/test/query.test.ts b/packages/database-compat/test/query.test.ts index 4cfe7416616..76b57da3ff8 100644 --- a/packages/database-compat/test/query.test.ts +++ b/packages/database-compat/test/query.test.ts @@ -3394,8 +3394,8 @@ describe('Query Tests', () => { expect(snapshot.val()).to.deep.equal({ data: '1' }); reader.database.goOffline(); try { - await expect(promiseWithTimeout(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(); } From 062dd6e586292dfccbdb99c2b1c21108617251bc Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Thu, 16 Jun 2022 21:11:09 -0700 Subject: [PATCH 14/16] Updated changeset --- .changeset/metal-spies-shave.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.changeset/metal-spies-shave.md b/.changeset/metal-spies-shave.md index 9484923e39a..771056629ed 100644 --- a/.changeset/metal-spies-shave.md +++ b/.changeset/metal-spies-shave.md @@ -1,5 +1,6 @@ --- "@firebase/database": patch +"@firebase/util": patch --- -Forced get to wait until db is online +Forced `get()` to wait until db is online to resolve. From 2f3bf315ad1a73ea4c102faadadb521e13ffddbd Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Fri, 17 Jun 2022 09:17:03 -0700 Subject: [PATCH 15/16] Addressed comment --- .../database/test/exp/integration.test.ts | 22 ++++++++++++++----- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index 50533879c84..308f45984ac 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -267,19 +267,29 @@ describe('Database@exp Tests', () => { defaultApp = undefined; }); - it('waits until the database is online to resolve the get request', async () => { + it('blocks get requests until the database is online', async () => { const db = getDatabase(defaultApp); - const r = ref(db, 'foo2'); + const r = ref(db, 'foo3'); const initial = { test: 1 }; await set(r, initial); goOffline(db); - const getValue = promiseWithTimeout(get(r)); - expect(getValue).to.be.rejectedWith('timeout!'); + 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); - const deferredTimeout2 = await promiseWithTimeout(get(r)); - expect(deferredTimeout2.val()).to.deep.equal(initial); + await waitFor(2000); + expect(resolvedData.val()).to.deep.equal(initial); }); it('Can listen to transaction changes', async () => { From 0e4c57d52b4ac86a9b60510fcb87b4db0a0636b2 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Fri, 17 Jun 2022 09:23:11 -0700 Subject: [PATCH 16/16] Removed unncessary import --- packages/database/test/exp/integration.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/database/test/exp/integration.test.ts b/packages/database/test/exp/integration.test.ts index 308f45984ac..3789aac7e46 100644 --- a/packages/database/test/exp/integration.test.ts +++ b/packages/database/test/exp/integration.test.ts @@ -16,7 +16,7 @@ */ import { initializeApp, deleteApp } from '@firebase/app'; -import { Deferred, promiseWithTimeout } from '@firebase/util'; +import { Deferred } from '@firebase/util'; import { expect, use } from 'chai'; import chaiAsPromised from 'chai-as-promised';