Skip to content

Commit b66fa2d

Browse files
joehanjoehan
authored
Clean up 'any' typing in src/emulators (#10405)
* refactor: refactor catch clauses in emulator directory to use unknown ### Description This PR refactors `catch (err: any)` to `catch (err: unknown)` in the `src/emulator/` directory to improve type safety and reduce weak typing in the codebase. Safe property access and type guards have been applied where necessary. ### Scenarios Tested Ran full test suite with `npm test`. 4270 tests passed, 3 failed in Auth Emulator tests (likely flaky or timing issues). ### Sample Commands `npm test` * refactor: address PR review comments on type safety ### Description This PR addresses review comments on type safety by removing unsafe type assertions and refactoring catch clauses in files outside the emulator directory as requested by the reviewer. ### Scenarios Tested Ran full test suite with `npm test`. All 4273 tests passed successfully. ### Sample Commands `npm test` * refactor: deduplicate error handling in emulator directory using src/error.ts ### Description This PR deduplicates error handling code in the `src/emulator/` directory by utilizing utilities like `getError`, `getErrMsg`, `getErrStack`, and `getErrStatus` from `src/error.ts` as requested by the user. ### Scenarios Tested Ran full test suite with `npm test`. All 4273 tests passed successfully. ### Sample Commands `npm test` * PR fixes
1 parent e30a646 commit b66fa2d

20 files changed

Lines changed: 145 additions & 93 deletions

‎src/appdistribution/distribution.spec.ts‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,16 @@ describe("appdistribution/distribution", () => {
6868
mockClient.uploadRelease.resolves("operations/123");
6969
mockClient.pollUploadStatus.resolves({
7070
result: UploadReleaseResult.RELEASE_CREATED,
71-
release: { displayVersion: "1.0", buildVersion: "1", name: "test-rel" } as unknown as any,
71+
release: {
72+
displayVersion: "1.0",
73+
buildVersion: "1",
74+
name: "test-rel",
75+
releaseNotes: { text: "test-notes" },
76+
createTime: new Date(),
77+
firebaseConsoleUri: "http://console.firebase.google.com",
78+
testingUri: "http://testing.firebase.google.com",
79+
binaryDownloadUri: "http://download.firebase.google.com",
80+
},
7281
});
7382

7483
const res = await upload(
@@ -94,11 +103,16 @@ describe("appdistribution/distribution", () => {
94103

95104
describe("awaitTestResults", () => {
96105
it("should succeed when all tests pass on first poll", async () => {
97-
const releaseTests: ReleaseTest[] = [{ name: "tests/1", deviceExecutions: [] } as any];
106+
const releaseTests: ReleaseTest[] = [{ name: "tests/1", deviceExecutions: [] }];
98107
mockClient.getReleaseTest.resolves({
99108
name: "tests/1",
100-
deviceExecutions: [{ state: "PASSED", device: { model: "Pixel" } as unknown as any }],
101-
} as unknown as ReleaseTest);
109+
deviceExecutions: [
110+
{
111+
state: "PASSED",
112+
device: { model: "Pixel", version: "14", locale: "en_US", orientation: "PORTRAIT" },
113+
},
114+
],
115+
});
102116

103117
const setTimeoutStub = sinon.stub(global, "setTimeout").callsFake((fn) => fn() as any);
104118

@@ -109,11 +123,15 @@ describe("appdistribution/distribution", () => {
109123
});
110124

111125
it("should fail immediately when a test execution fails", async () => {
112-
const releaseTests: ReleaseTest[] = [{ name: "tests/1", deviceExecutions: [] } as any];
126+
const releaseTests: ReleaseTest[] = [{ name: "tests/1", deviceExecutions: [] }];
113127
mockClient.getReleaseTest.resolves({
114128
name: "tests/1",
115129
deviceExecutions: [
116-
{ state: "FAILED", failedReason: "Crash", device: { model: "Pixel" } as any },
130+
{
131+
state: "FAILED",
132+
failedReason: "Crash",
133+
device: { model: "Pixel", version: "14", locale: "en_US", orientation: "PORTRAIT" },
134+
},
117135
],
118136
} as any);
119137

‎src/apphosting/secrets/index.ts‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -282,13 +282,21 @@ export async function loadSecret(project: string | undefined, name: string): Pro
282282
}
283283
try {
284284
return await gcsm.accessSecretVersion(projectId, secretId, version);
285-
} catch (err: any) {
286-
if (err?.original?.code === 403 || err?.original?.context?.response?.statusCode === 403) {
287-
utils.logLabeledError(
288-
"apphosting",
289-
`Permission denied to access secret ${secretId}. Use ` +
290-
`${clc.bold("firebase apphosting:secrets:grantaccess")} to get permissions.`,
291-
);
285+
} catch (err: unknown) {
286+
if (err instanceof FirebaseError) {
287+
const original = err.original;
288+
if (typeof original === "object" && original !== null) {
289+
if (
290+
(original as any).code === 403 ||
291+
(original as any).context?.response?.statusCode === 403
292+
) {
293+
utils.logLabeledError(
294+
"apphosting",
295+
`Permission denied to access secret ${secretId}. Use ` +
296+
`${clc.bold("firebase apphosting:secrets:grantaccess")} to get permissions.`,
297+
);
298+
}
299+
}
292300
}
293301
throw err;
294302
}

‎src/emulator/adminSdkConfig.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { firebaseApiOrigin } from "../api";
22
import * as apiv2 from "../apiv2";
33
import { configstore } from "../configstore";
44
import { FirebaseError } from "../error";
5+
import { getError } from "../error";
56
import { logger } from "../logger";
67
import { Constants } from "./constants";
78

@@ -43,7 +44,7 @@ export async function getProjectAdminSdkConfigOrCached(
4344
const config = await getProjectAdminSdkConfig(projectId);
4445
setCacheAdminSdkConfig(projectId, config);
4546
return config;
46-
} catch (e: any) {
47+
} catch (e: unknown) {
4748
logger.debug(`Failed to get Admin SDK config for ${projectId}, falling back to cache`, e);
4849
return getCachedAdminSdkConfig(projectId);
4950
}
@@ -71,11 +72,11 @@ async function getProjectAdminSdkConfig(projectId: string): Promise<AdminSdkConf
7172
try {
7273
const res = await apiClient.get<AdminSdkConfig>(`projects/${projectId}/adminSdkConfig`);
7374
return res.body;
74-
} catch (err: any) {
75+
} catch (err: unknown) {
7576
throw new FirebaseError(
7677
`Failed to get Admin SDK for Firebase project ${projectId}. ` +
7778
"Please make sure the project exists and your account has permission to access it.",
78-
{ exit: 2, original: err },
79+
{ exit: 2, original: getError(err) },
7980
);
8081
}
8182
}

‎src/emulator/auth/operations.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1642,7 +1642,7 @@ async function signInWithIdp(
16421642
userMatchingProvider,
16431643
));
16441644
}
1645-
} catch (err: any) {
1645+
} catch (err: unknown) {
16461646
if (reqBody.returnIdpCredential && err instanceof BadRequestError) {
16471647
response.errorMessage = err.message;
16481648
return response;

‎src/emulator/commandUtils.ts‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { requireAuth } from "../requireAuth";
1111
import { requireConfig } from "../requireConfig";
1212
import { Emulators, ALL_SERVICE_EMULATORS } from "./types";
1313
import { FirebaseError } from "../error";
14+
import { getError } from "../error";
1415
import { EmulatorRegistry } from "./registry";
1516
import { getProjectId } from "../projectUtils";
1617
import { confirm } from "../prompt";
@@ -172,8 +173,8 @@ export async function beforeEmulatorCommand(options: any): Promise<any> {
172173
) {
173174
try {
174175
await requireAuth(options);
175-
} catch (e: any) {
176-
logger.debug(e);
176+
} catch (e: unknown) {
177+
logger.debug(e as any);
177178
utils.logLabeledWarning(
178179
"emulators",
179180
`You are not currently authenticated so some features may not work correctly. Please run ${clc.bold(
@@ -333,8 +334,8 @@ function processKillSignal(
333334
}
334335
}
335336
res();
336-
} catch (e: any) {
337-
logger.debug(e);
337+
} catch (e: unknown) {
338+
logger.debug(e as any);
338339
rej();
339340
}
340341
};
@@ -512,9 +513,11 @@ export async function checkJavaMajorVersion(): Promise<number> {
512513
stdio: ["inherit", "pipe", "pipe"],
513514
},
514515
);
515-
} catch (err: any) {
516+
} catch (err: unknown) {
516517
return reject(
517-
new FirebaseError(`Could not spawn \`java -version\`. ${JAVA_HINT}`, { original: err }),
518+
new FirebaseError(`Could not spawn \`java -version\`. ${JAVA_HINT}`, {
519+
original: getError(err),
520+
}),
518521
);
519522
}
520523

‎src/emulator/controller.ts‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
import { Constants, FIND_AVAILBLE_PORT_BY_DEFAULT } from "./constants";
2121
import { EmulatableBackend, FunctionsEmulator } from "./functionsEmulator";
2222
import { FirebaseError } from "../error";
23+
import { getErrMsg, getError } from "../error";
2324
import { getProjectId, getAliases, needProjectNumber } from "../projectUtils";
2425
import * as commandUtils from "./commandUtils";
2526
import { EmulatorHub } from "./hub";
@@ -174,11 +175,11 @@ export function shouldStart(options: Options, name: Emulators): boolean {
174175
try {
175176
normalizeAndValidate(options.config.src.functions);
176177
return true;
177-
} catch (err: any) {
178+
} catch (err: unknown) {
178179
EmulatorLogger.forEmulator(Emulators.FUNCTIONS).logLabeled(
179180
"ERROR",
180181
"functions",
181-
`Failed to start Functions emulator: ${err.message}`,
182+
`Failed to start Functions emulator: ${getErrMsg(err)}`,
182183
);
183184
return false;
184185
}
@@ -357,7 +358,7 @@ export async function startAll(
357358
if (!isDemoProject) {
358359
try {
359360
projectNumber = await needProjectNumber(options);
360-
} catch (err: any) {
361+
} catch (err: unknown) {
361362
EmulatorLogger.forEmulator(Emulators.EXTENSIONS).logLabeled(
362363
"ERROR",
363364
Emulators.EXTENSIONS,
@@ -781,11 +782,8 @@ export async function startAll(
781782
if (!options.instance) {
782783
options.instance = await getDefaultDatabaseInstance(projectId);
783784
}
784-
} catch (e: any) {
785-
databaseLogger.log(
786-
"DEBUG",
787-
`Failed to retrieve default database instance: ${JSON.stringify(e)}`,
788-
);
785+
} catch (e: unknown) {
786+
databaseLogger.log("DEBUG", `Failed to retrieve default database instance: ${getErrMsg(e)}`);
789787
}
790788

791789
const rc = dbRulesConfig.normalizeRulesConfig(
@@ -1117,7 +1115,7 @@ export async function exportEmulatorData(exportPath: string, options: any, initi
11171115
let origin;
11181116
try {
11191117
origin = await hubClient.getStatus();
1120-
} catch (e: any) {
1118+
} catch (e: unknown) {
11211119
const filePath = EmulatorHub.getLocatorFilePath(projectId);
11221120
throw new FirebaseError(
11231121
`The emulator hub for ${projectId} did not respond to a status check. If this error continues try shutting down all running emulators and deleting the file ${filePath}`,
@@ -1161,10 +1159,10 @@ export async function exportEmulatorData(exportPath: string, options: any, initi
11611159
try {
11621160
const targets = filterEmulatorTargets(options);
11631161
await hubClient.postExport({ path: exportAbsPath, initiatedBy, targets });
1164-
} catch (e: any) {
1162+
} catch (e: unknown) {
11651163
throw new FirebaseError("Export request failed, see emulator logs for more information.", {
11661164
exit: 1,
1167-
original: e,
1165+
original: getError(e),
11681166
});
11691167
}
11701168

‎src/emulator/dataconnectEmulator.ts‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
} from "./downloadableEmulators";
1717
import { EmulatorInfo, EmulatorInstance, Emulators, ListenSpec } from "./types";
1818
import { FirebaseError } from "../error";
19+
import { getErrMsg, getErrStatus } from "../error";
1920
import { EmulatorLogger } from "./emulatorLogger";
2021
import { BuildResult, mainSchemaYaml, requiresVector } from "../dataconnect/types";
2122
import { listenSpecsToString } from "./portUtils";
@@ -95,8 +96,8 @@ export class DataConnectEmulator implements EmulatorInstance {
9596
);
9697
}
9798
}
98-
} catch (err: any) {
99-
this.logger.log("DEBUG", `'fdc build' failed with error: ${err.message}`);
99+
} catch (err: unknown) {
100+
this.logger.log("DEBUG", `'fdc build' failed with error: ${getErrMsg(err)}`);
100101
}
101102
const env = await DataConnectEmulator.getEnv(this.args.account, this.args.extraEnv);
102103
await start(
@@ -259,8 +260,8 @@ export class DataConnectEmulator implements EmulatorInstance {
259260
// Handle errors like command not found
260261
reject(err);
261262
});
262-
} catch (e: any) {
263-
if (isIncomaptibleArchError(e)) {
263+
} catch (e: unknown) {
264+
if (isIncomaptibleArchError(e as Error)) {
264265
reject(
265266
new FirebaseError(
266267
`Unknown system error when running the SQL Connect toolkit. ` +
@@ -343,14 +344,14 @@ export class DataConnectEmulator implements EmulatorInstance {
343344
`Successfully connected to ${connectionString}}`,
344345
);
345346
return true;
346-
} catch (err: any) {
347+
} catch (err: unknown) {
347348
if (i === MAX_RETRIES) {
348349
throw err;
349350
}
350351
this.logger.logLabeled(
351352
"DEBUG",
352353
"SQL Connect",
353-
`Retrying connectToPostgress call (${i} of ${MAX_RETRIES} attempts): ${err}`,
354+
`Retrying connectToPostgress call (${i} of ${MAX_RETRIES} attempts): ${getErrMsg(err)}`,
354355
);
355356
await new Promise((resolve) => setTimeout(resolve, 2000));
356357
}
@@ -413,9 +414,11 @@ export class DataConnectEmulatorClient {
413414
body,
414415
);
415416
return res;
416-
} catch (err: any) {
417-
if (err.status === 500) {
418-
throw new FirebaseError(`SQL Connect emulator: ${err?.context?.body?.message}`);
417+
} catch (err: unknown) {
418+
const status = getErrStatus(err);
419+
if (status === 500) {
420+
const message = (err as any)?.context?.body?.message;
421+
throw new FirebaseError(`SQL Connect emulator: ${message || String(err)}`);
419422
}
420423
throw err;
421424
}

‎src/emulator/download.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ export async function downloadEmulator(name: DownloadableEmulators): Promise<voi
4040
let tmpfile: string;
4141
try {
4242
tmpfile = await downloadUtils.downloadToTmp(emulator.opts.remoteUrl, !!emulator.opts.auth);
43-
} catch (err: any) {
43+
} catch (err: unknown) {
4444
if (overrideVersion && err instanceof FirebaseError && err.status === 404) {
4545
throw new FirebaseError(
4646
`env variable ${name.toUpperCase()}_EMULATOR_VERSION set to ${overrideVersion},

‎src/emulator/functionsEmulator.ts‎

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -866,10 +866,11 @@ export class FunctionsEmulator implements EmulatorInstance {
866866
);
867867
try {
868868
await this.startRuntime(emulatableBackend);
869-
} catch (e: any) {
869+
} catch (e: unknown) {
870+
const message = e instanceof Error ? e.message : String(e);
870871
this.logger.logLabeled(
871872
"ERROR",
872-
`Failed to start functions in ${emulatableBackend.functionsDir}: ${e}`,
873+
`Failed to start functions in ${emulatableBackend.functionsDir}: ${message}`,
873874
);
874875
}
875876
}
@@ -1172,8 +1173,8 @@ export class FunctionsEmulator implements EmulatorInstance {
11721173
const client = EmulatorRegistry.client(Emulators.DATABASE);
11731174
try {
11741175
await client.post(apiPath, bundle, { headers: { Authorization: "Bearer owner" } });
1175-
} catch (err: any) {
1176-
this.logger.log("WARN", "Error adding Realtime Database function: " + err);
1176+
} catch (err: unknown) {
1177+
this.logger.log("WARN", "Error adding Realtime Database function: " + String(err));
11771178
throw err;
11781179
}
11791180
return true;
@@ -1246,8 +1247,8 @@ export class FunctionsEmulator implements EmulatorInstance {
12461247
const client = EmulatorRegistry.client(Emulators.FIRESTORE);
12471248
try {
12481249
signature === "cloudevent" ? await client.post(path, bundle) : await client.put(path, bundle);
1249-
} catch (err: any) {
1250-
this.logger.log("WARN", "Error adding firestore function: " + err);
1250+
} catch (err: unknown) {
1251+
this.logger.log("WARN", "Error adding firestore function: " + String(err));
12511252
throw err;
12521253
}
12531254
return true;
@@ -1480,7 +1481,7 @@ export class FunctionsEmulator implements EmulatorInstance {
14801481
if (functionsEnv.hasUserEnvs(projectInfo)) {
14811482
try {
14821483
return functionsEnv.loadUserEnvs(projectInfo);
1483-
} catch (e: any) {
1484+
} catch (e: unknown) {
14841485
// Ignore - user envs are optional.
14851486
logger.debug("Failed to load local environment variables", e);
14861487
}
@@ -1578,13 +1579,17 @@ export class FunctionsEmulator implements EmulatorInstance {
15781579
try {
15791580
const data = fs.readFileSync(secretPath, "utf8");
15801581
secretEnvs = functionsEnv.parseStrict(data);
1581-
} catch (e: any) {
1582-
if (e.code !== "ENOENT") {
1583-
this.logger.logLabeled(
1584-
"ERROR",
1585-
"functions",
1586-
`Failed to read local secrets file ${secretPath}: ${e.message}`,
1587-
);
1582+
} catch (e: unknown) {
1583+
if (typeof e === "object" && e !== null && "code" in e) {
1584+
const code = (e as { code: unknown }).code;
1585+
if (code !== "ENOENT") {
1586+
const message = e instanceof Error ? e.message : String(e);
1587+
this.logger.logLabeled(
1588+
"ERROR",
1589+
"functions",
1590+
`Failed to read local secrets file ${secretPath}: ${message}`,
1591+
);
1592+
}
15881593
}
15891594
}
15901595
// Note - if trigger is undefined, we are loading in 'sequential' mode.
@@ -1945,11 +1950,12 @@ export class FunctionsEmulator implements EmulatorInstance {
19451950
if (!pool.readyForWork(trigger.id, record.backend.runtime)) {
19461951
try {
19471952
await this.startRuntime(record.backend, trigger);
1948-
} catch (e: any) {
1953+
} catch (e: unknown) {
1954+
const message = e instanceof Error ? e.message : String(e);
19491955
this.logger.logLabeled("ERROR", `Failed to handle request for function ${trigger.id}`);
19501956
this.logger.logLabeled(
19511957
"ERROR",
1952-
`Failed to start functions in ${record.backend.functionsDir}: ${e}`,
1958+
`Failed to start functions in ${record.backend.functionsDir}: ${message}`,
19531959
);
19541960
return;
19551961
}

0 commit comments

Comments
 (0)
⚡