Skip to content

Commit 2e214ed

Browse files
inlinedmbleigh
andauthored
Bug bash fixes (#933)
* HTTP functions always return a promise even when apply CORS. Previously HTTPS functions had to be patched in the emulator to ensure that the promise is always handled. This change should circumvent that need and make v2 patching unnecessary. * Export undocumented dependency of the emulator * Make @types/cors a proper dep * Add dummy files to silence imports eslinter Co-authored-by: Michael Bleigh <[email protected]>
1 parent e2ccc53 commit 2e214ed

File tree

15 files changed

+281
-8
lines changed

15 files changed

+281
-8
lines changed

logger/compat.js

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// The MIT License (MIT)
2+
//
3+
// Copyright (c) 2021 Firebase
4+
//
5+
// Permission is hereby granted, free of charge, to any person obtaining a copy
6+
// of this software and associated documentation files (the "Software"), to deal
7+
// in the Software without restriction, including without limitation the rights
8+
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
9+
// copies of the Software, and to permit persons to whom the Software is
10+
// furnished to do so, subject to the following conditions:
11+
//
12+
// The above copyright notice and this permission notice shall be included in all
13+
// copies or substantial portions of the Software.
14+
//
15+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
16+
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
17+
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
18+
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
19+
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
20+
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
21+
// SOFTWARE.
22+
23+
// This file is not part of the firebase-functions SDK. It is used to silence the
24+
// imports eslint plugin until it can understand import paths defined by node
25+
// package exports.
26+
// For more information, see github.com/import-js/eslint-plugin-import/issues/1810

logger/index.js

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// The MIT License (MIT)
2+
//
3+
// Copyright (c) 2021 Firebase
4+
//
5+
// Permission is hereby granted, free of charge, to any person obtaining a copy
6+
// of this software and associated documentation files (the "Software"), to deal
7+
// in the Software without restriction, including without limitation the rights
8+
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
9+
// copies of the Software, and to permit persons to whom the Software is
10+
// furnished to do so, subject to the following conditions:
11+
//
12+
// The above copyright notice and this permission notice shall be included in all
13+
// copies or substantial portions of the Software.
14+
//
15+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
16+
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
17+
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
18+
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
19+
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
20+
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
21+
// SOFTWARE.
22+
23+
// This file is not part of the firebase-functions SDK. It is used to silence the
24+
// imports eslint plugin until it can understand import paths defined by node
25+
// package exports.
26+
// For more information, see github.com/import-js/eslint-plugin-import/issues/1810

package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
"types": "lib/index.d.ts",
2626
"exports": {
2727
".": "./lib/index.js",
28+
"./lib/providers/https": "./lib/v1/providers/https.js",
2829
"./v1": "./lib/v1/index.js",
2930
"./logger": "./lib/logger/index.js",
3031
"./logger/compat": "./lib/logger/compat.js",
@@ -84,6 +85,7 @@
8485
"test": "mocha"
8586
},
8687
"dependencies": {
88+
"@types/cors": "^2.8.5",
8789
"@types/express": "4.17.3",
8890
"cors": "^2.8.5",
8991
"express": "^4.17.1",
@@ -92,7 +94,6 @@
9294
"devDependencies": {
9395
"@types/chai": "^4.1.7",
9496
"@types/chai-as-promised": "^7.1.0",
95-
"@types/cors": "^2.8.5",
9697
"@types/jsonwebtoken": "^8.3.2",
9798
"@types/lodash": "^4.14.135",
9899
"@types/mocha": "^5.2.7",

spec/common/providers/https.spec.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ function runHandler(
5858
class MockResponse {
5959
private statusCode = 0;
6060
private headers: { [name: string]: string } = {};
61+
private callback: Function;
6162

6263
public status(code: number) {
6364
this.statusCode = code;
@@ -79,11 +80,21 @@ function runHandler(
7980
headers: this.headers,
8081
body,
8182
});
83+
if (this.callback) {
84+
this.callback();
85+
}
8286
}
8387

8488
public end() {
8589
this.send(undefined);
8690
}
91+
92+
public on(event: string, callback: Function) {
93+
if (event !== 'finish') {
94+
throw new Error('MockResponse only implements the finish event');
95+
}
96+
this.callback = callback;
97+
}
8798
}
8899

89100
const response = new MockResponse();

spec/v1/providers/https.spec.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ function runHandler(
4949
class MockResponse {
5050
private statusCode = 0;
5151
private headers: { [name: string]: string } = {};
52+
private callback: Function;
5253

5354
public status(code: number) {
5455
this.statusCode = code;
@@ -70,13 +71,22 @@ function runHandler(
7071
headers: this.headers,
7172
body,
7273
});
74+
if (this.callback) {
75+
this.callback();
76+
}
7377
}
7478

7579
public end() {
7680
this.send(undefined);
7781
}
78-
}
7982

83+
public on(event: string, callback: Function) {
84+
if (event !== 'finish') {
85+
throw new Error('MockResponse only implements the finish event');
86+
}
87+
this.callback = callback;
88+
}
89+
}
8090
const response = new MockResponse();
8191
handler(request, response as any, () => undefined);
8292
});

spec/v2/providers/https.spec.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ function runHandler(
2828
class MockResponse {
2929
private statusCode = 0;
3030
private headers: { [name: string]: string } = {};
31+
private callback: Function;
3132

3233
public status(code: number) {
3334
this.statusCode = code;
@@ -49,11 +50,21 @@ function runHandler(
4950
headers: this.headers,
5051
body,
5152
});
53+
if (this.callback) {
54+
this.callback();
55+
}
5256
}
5357

5458
public end() {
5559
this.send(undefined);
5660
}
61+
62+
public on(event: string, callback: Function) {
63+
if (event !== 'finish') {
64+
throw new Error('MockResponse only implements the finish event');
65+
}
66+
this.callback = callback;
67+
}
5768
}
5869

5970
const response = new MockResponse();

src/common/providers/https.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -598,9 +598,12 @@ export function onCallHandler<Req = any, Res = any>(
598598
handler: v1Handler | v2Handler<Req, Res>
599599
): (req: Request, res: express.Response) => Promise<void> {
600600
const wrapped = wrapOnCallHandler(handler);
601-
return async (req: Request, res: express.Response) => {
602-
cors(options)(req, res, () => {
603-
return wrapped(req, res);
601+
return (req: Request, res: express.Response) => {
602+
return new Promise((resolve) => {
603+
res.on('finish', resolve);
604+
cors(options)(req, res, () => {
605+
resolve(wrapped(req, res));
606+
});
604607
});
605608
};
606609
}

src/v2/providers/https.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,12 @@ export function onRequest(
8383

8484
if ('cors' in opts) {
8585
const userProvidedHandler = handler;
86-
handler = (req: Request, res: express.Response) => {
87-
cors({ origin: opts.cors })(req, res, () => {
88-
userProvidedHandler(req, res);
86+
handler = (req: Request, res: express.Response): void | Promise<void> => {
87+
return new Promise((resolve) => {
88+
res.on('finish', resolve);
89+
cors({ origin: opts.cors })(req, res, () => {
90+
resolve(userProvidedHandler(req, res));
91+
});
8992
});
9093
};
9194
}

v1/index.js

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// The MIT License (MIT)
2+
//
3+
// Copyright (c) 2021 Firebase
4+
//
5+
// Permission is hereby granted, free of charge, to any person obtaining a copy
6+
// of this software and associated documentation files (the "Software"), to deal
7+
// in the Software without restriction, including without limitation the rights
8+
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
9+
// copies of the Software, and to permit persons to whom the Software is
10+
// furnished to do so, subject to the following conditions:
11+
//
12+
// The above copyright notice and this permission notice shall be included in all
13+
// copies or substantial portions of the Software.
14+
//
15+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
16+
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
17+
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
18+
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
19+
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
20+
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
21+
// SOFTWARE.
22+
23+
// This file is not part of the firebase-functions SDK. It is used to silence the
24+
// imports eslint plugin until it can understand import paths defined by node
25+
// package exports.
26+
// For more information, see github.com/import-js/eslint-plugin-import/issues/1810

v2/core.js

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// The MIT License (MIT)
2+
//
3+
// Copyright (c) 2021 Firebase
4+
//
5+
// Permission is hereby granted, free of charge, to any person obtaining a copy
6+
// of this software and associated documentation files (the "Software"), to deal
7+
// in the Software without restriction, including without limitation the rights
8+
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
9+
// copies of the Software, and to permit persons to whom the Software is
10+
// furnished to do so, subject to the following conditions:
11+
//
12+
// The above copyright notice and this permission notice shall be included in all
13+
// copies or substantial portions of the Software.
14+
//
15+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
16+
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
17+
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
18+
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
19+
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
20+
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
21+
// SOFTWARE.
22+
23+
// This file is not part of the firebase-functions SDK. It is used to silence the
24+
// imports eslint plugin until it can understand import paths defined by node
25+
// package exports.
26+
// For more information, see github.com/import-js/eslint-plugin-import/issues/1810

0 commit comments

Comments
 (0)