Skip to content

Commit

Permalink
clear failAfter timeout after request resolves (#3330)
Browse files Browse the repository at this point in the history
* clear failAfter timeout after request resolves

* Create nasty-pigs-invite.md

* add a test

* fix test setup issues
  • Loading branch information
Feiyang1 committed Jul 7, 2020
1 parent 26767cd commit bb74083
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 11 deletions.
6 changes: 6 additions & 0 deletions .changeset/nasty-pigs-invite.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@firebase/functions": patch
"firebase": patch
---

Clear timeout after a successful response or after the request is canceled. Fixes [issue 3289](https://github.com/firebase/firebase-js-sdk/issues/3289).
2 changes: 1 addition & 1 deletion config/functions/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@
},
"private": true,
"engines": {
"node": "8"
"node": "10"
}
}
35 changes: 29 additions & 6 deletions packages/functions/src/api/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,23 @@ export interface HttpResponseBody {
*
* @param millis Number of milliseconds to wait before rejecting.
*/
function failAfter(millis: number): Promise<never> {
return new Promise((_, reject) => {
setTimeout(() => {
function failAfter(
millis: number
): {
timer: number | NodeJS.Timeout;
promise: Promise<never>;
} {
let timer!: number | NodeJS.Timeout;
const promise = new Promise<never>((_, reject) => {
timer = setTimeout(() => {
reject(new HttpsErrorImpl('deadline-exceeded', 'deadline-exceeded'));
}, millis);
});

return {
timer,
promise
};
}

/**
Expand Down Expand Up @@ -213,10 +224,12 @@ export class Service implements FirebaseFunctions, FirebaseService {
// Default timeout to 70s, but let the options override it.
const timeout = options.timeout || 70000;

const { timer, promise: failAfterPromise } = failAfter(timeout);

const response = await Promise.race([
this.postJSON(url, body, headers),
failAfter(timeout),
this.cancelAllRequests
clearTimeoutWrapper(timer, this.postJSON(url, body, headers)),
failAfterPromise,
clearTimeoutWrapper(timer, this.cancelAllRequests)
]);

// If service was deleted, interrupted response throws an error.
Expand Down Expand Up @@ -261,3 +274,13 @@ export class Service implements FirebaseFunctions, FirebaseService {
return { data: decodedData };
}
}

async function clearTimeoutWrapper<T>(
timer: number | NodeJS.Timeout,
promise: Promise<T>
): Promise<T> {
const result = await promise;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
clearTimeout(timer as any);
return result;
}
10 changes: 10 additions & 0 deletions packages/functions/test/callable.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,4 +190,14 @@ describe('Firebase Functions > Call', () => {
const func = functions.httpsCallable('timeoutTest', { timeout: 10 });
await expectError(func(), 'deadline-exceeded', 'deadline-exceeded');
});

it('cancels timeout', async () => {
const functions = createTestService(app, region);
const globalObj = typeof window !== 'undefined' ? window : global;
const clearTimeoutSpy = sinon.spy(globalObj, 'clearTimeout');
const func = functions.httpsCallable('nullTest');
await func(null);
expect(clearTimeoutSpy.called).to.be.true;
clearTimeoutSpy.restore();
});
});
8 changes: 4 additions & 4 deletions tools/config.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/**
* @license
* Copyright 2017 Google Inc.
* Copyright 2017 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -72,12 +72,12 @@ Promise.resolve(userToken || cachedToken)
})();

// Write config to top-level config directory
await firebaseTools.setup
.web({ project, token })
await firebaseTools.apps
.sdkconfig('web', undefined, { project, token })
.then(config =>
fs.writeFile(
path.resolve(__dirname, '../config/project.json'),
JSON.stringify(config, null, 2)
JSON.stringify(config.sdkConfig, null, 2)
)
);

Expand Down

0 comments on commit bb74083

Please sign in to comment.