Skip to content

Commit 53603f3

Browse files
committed
Implement solution to replace error messages with error codes, instead of removing error messages.
1 parent a90255a commit 53603f3

35 files changed

+241
-139
lines changed

packages/firestore/scripts/build-bundle.js

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/firestore/scripts/remove-asserts.js

Lines changed: 0 additions & 17 deletions
This file was deleted.

packages/firestore/scripts/remove-asserts.ts

Lines changed: 72 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,15 @@
1515
* limitations under the License.
1616
*/
1717

18+
import { createHash } from 'crypto';
19+
import { existsSync, readFileSync, writeFileSync } from 'fs';
20+
import { join } from 'path';
21+
1822
import * as ts from 'typescript';
1923

2024
// Location of file that includes the asserts
2125
const ASSERT_LOCATION = 'packages/firestore/src/util/assert.ts';
26+
const ERROR_CODE_LOCATION = '../dist/error_codes.json';
2227

2328
export function removeAsserts(
2429
program: ts.Program
@@ -31,7 +36,8 @@ export function removeAsserts(
3136

3237
/**
3338
* Transformer that removes all "debugAssert" statements from the SDK and
34-
* removes the custom message for fail() and hardAssert().
39+
* replaces the custom message for fail() and hardAssert() with shorter
40+
* error codes
3541
*/
3642
class RemoveAsserts {
3743
constructor(private readonly typeChecker: ts.TypeChecker) {}
@@ -63,22 +69,37 @@ class RemoveAsserts {
6369
declaration.getSourceFile().fileName.indexOf(ASSERT_LOCATION) >= 0
6470
) {
6571
const method = declaration.name!.text;
72+
6673
if (method === 'debugAssert') {
6774
updatedNode = ts.createOmittedExpression();
68-
} else if (method === 'hardAssert') {
69-
// Remove the log message but keep the assertion
70-
updatedNode = ts.createCall(
71-
declaration.name!,
72-
/*typeArgs*/ undefined,
73-
[node.arguments[0]]
74-
);
75-
} else if (method === 'fail') {
76-
// Remove the log message
77-
updatedNode = ts.createCall(
78-
declaration.name!,
79-
/*typeArgs*/ undefined,
80-
[]
81-
);
75+
} else if ((method === 'hardAssert') || (method === 'fail')) {
76+
const messageIndex = (method === 'hardAssert') ? 1 : 0;
77+
if ((node.arguments.length > messageIndex) && (node.arguments[messageIndex].kind === ts.SyntaxKind.StringLiteral)) {
78+
const stringLiteral: ts.StringLiteral = node.arguments[messageIndex] as ts.StringLiteral;
79+
const errorMessage = RemoveAsserts.trimErrorMessage(stringLiteral.getFullText());
80+
const errorCode = RemoveAsserts.errorCode(errorMessage);
81+
82+
RemoveAsserts.saveErrorCode(errorCode, errorMessage);
83+
const newArguments = [...node.arguments];
84+
newArguments[messageIndex] = ts.createLiteral(errorCode);
85+
86+
// Replace the call with the full error message to a
87+
// build with an error code
88+
updatedNode = ts.createCall(
89+
declaration.name!,
90+
/*typeArgs*/ undefined,
91+
newArguments
92+
);
93+
} else {
94+
const newArguments = [...node.arguments];
95+
newArguments[messageIndex] = ts.createLiteral('Unexpected error');
96+
// Remove the log message but keep the assertion
97+
updatedNode = ts.createCall(
98+
declaration.name!,
99+
/*typeArgs*/ undefined,
100+
newArguments
101+
);
102+
}
82103
}
83104
}
84105
}
@@ -91,4 +112,40 @@ class RemoveAsserts {
91112
return node;
92113
}
93114
}
115+
116+
static trimErrorMessage(errorMessage: string): string {
117+
return errorMessage.substring(
118+
errorMessage.indexOf("'") + 1,
119+
errorMessage.lastIndexOf("'"));
120+
}
121+
122+
static errorCode(errorMessage: string): string {
123+
// Create a sha256 hash from the parameter names and types.
124+
const hash = createHash('sha256');
125+
hash.update(errorMessage);
126+
127+
// Use the first 7 characters of the hash for a more compact code.
128+
const paramHash = hash.digest('hex').substring(0, 7);
129+
130+
return paramHash;
131+
}
132+
133+
static saveErrorCode(errorCode: string, errorMessage: string): void {
134+
const errorCodes = RemoveAsserts.getErrorCodes();
135+
errorCodes[errorCode] = errorMessage;
136+
RemoveAsserts.saveErrorCodes(errorCodes);
137+
}
138+
139+
static getErrorCodes(): Record<string, string> {
140+
const path = join(module.path, ERROR_CODE_LOCATION);
141+
if (!existsSync(path)){
142+
return {};
143+
}
144+
return JSON.parse(readFileSync(path, 'utf-8'));
145+
}
146+
147+
static saveErrorCodes(errorCodes: Record<string, string>): void {
148+
const path = join(module.path, ERROR_CODE_LOCATION);
149+
writeFileSync(path, JSON.stringify(errorCodes, undefined, 4), {encoding: "utf-8", });
150+
}
94151
}

packages/firestore/src/api/credentials.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,8 @@ export class LiteAuthCredentialsProvider implements CredentialsProvider<User> {
206206
if (tokenData) {
207207
hardAssert(
208208
typeof tokenData.accessToken === 'string',
209-
'Invalid tokenData returned from getToken():' + tokenData
209+
'Invalid tokenData returned from getToken()',
210+
{ tokenData }
210211
);
211212
return new OAuthToken(
212213
tokenData.accessToken,
@@ -350,7 +351,8 @@ export class FirebaseAuthCredentialsProvider
350351
if (tokenData) {
351352
hardAssert(
352353
typeof tokenData.accessToken === 'string',
353-
'Invalid tokenData returned from getToken():' + tokenData
354+
'Invalid tokenData returned from getToken()',
355+
{ tokenData }
354356
);
355357
return new OAuthToken(tokenData.accessToken, this.currentUser);
356358
} else {
@@ -378,7 +380,8 @@ export class FirebaseAuthCredentialsProvider
378380
const currentUid = this.auth && this.auth.getUid();
379381
hardAssert(
380382
currentUid === null || typeof currentUid === 'string',
381-
'Received invalid UID: ' + currentUid
383+
'Received invalid UID',
384+
{ currentUid }
382385
);
383386
return new User(currentUid);
384387
}
@@ -564,7 +567,8 @@ export class FirebaseAppCheckTokenProvider
564567
if (tokenResult) {
565568
hardAssert(
566569
typeof tokenResult.token === 'string',
567-
'Invalid tokenResult returned from getToken():' + tokenResult
570+
'Invalid tokenResult returned from getToken()',
571+
{ tokenResult }
568572
);
569573
this.latestAppCheckToken = tokenResult.token;
570574
return new AppCheckToken(tokenResult.token);
@@ -625,7 +629,8 @@ export class LiteAppCheckTokenProvider implements CredentialsProvider<string> {
625629
if (tokenResult) {
626630
hardAssert(
627631
typeof tokenResult.token === 'string',
628-
'Invalid tokenResult returned from getToken():' + tokenResult
632+
'Invalid tokenResult returned from getToken()',
633+
{ tokenResult }
629634
);
630635
return new AppCheckToken(tokenResult.token);
631636
} else {

packages/firestore/src/api/snapshot.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -749,7 +749,7 @@ export function resultChangeType(type: ChangeType): DocumentChangeType {
749749
case ChangeType.Removed:
750750
return 'removed';
751751
default:
752-
return fail('Unknown change type: ' + type);
752+
return fail('Unknown change type', { type });
753753
}
754754
}
755755

packages/firestore/src/core/filter.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ export class FieldFilter extends Filter {
168168
case Operator.GREATER_THAN_OR_EQUAL:
169169
return comparison >= 0;
170170
default:
171-
return fail('Unknown FieldFilter operator: ' + this.op);
171+
return fail('Unknown FieldFilter operator', { operator: this.op });
172172
}
173173
}
174174

packages/firestore/src/core/query.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -578,6 +578,6 @@ export function compareDocs(
578578
case Direction.DESCENDING:
579579
return -1 * comparison;
580580
default:
581-
return fail('Unknown direction: ' + orderBy.dir);
581+
return fail('Unknown direction', { direction: orderBy.dir });
582582
}
583583
}

packages/firestore/src/core/sync_engine_impl.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -992,7 +992,7 @@ function updateTrackedLimbos(
992992
removeLimboTarget(syncEngineImpl, limboChange.key);
993993
}
994994
} else {
995-
fail('Unknown limbo change: ' + JSON.stringify(limboChange));
995+
fail('Unknown limbo change', { limboChange });
996996
}
997997
}
998998
}
@@ -1309,7 +1309,7 @@ export async function syncEngineApplyBatchState(
13091309
batchId
13101310
);
13111311
} else {
1312-
fail(`Unknown batchState: ${batchState}`);
1312+
fail(`Unknown batchState`, { batchState });
13131313
}
13141314

13151315
await syncEngineEmitNewSnapsAndNotifyLocalStore(syncEngineImpl, documents);
@@ -1552,7 +1552,7 @@ export async function syncEngineApplyTargetState(
15521552
break;
15531553
}
15541554
default:
1555-
fail('Unexpected target state: ' + state);
1555+
fail('Unexpected target state', state);
15561556
}
15571557
}
15581558
}

packages/firestore/src/core/transaction.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,9 @@ export class Transaction {
124124
// Represent a deleted doc using SnapshotVersion.min().
125125
docVersion = SnapshotVersion.min();
126126
} else {
127-
throw fail('Document in a transaction was a ' + doc.constructor.name);
127+
throw fail('Document in a transaction was a ', {
128+
documentName: doc.constructor.name
129+
});
128130
}
129131

130132
const existingVersion = this.readVersions.get(doc.key.toString());

packages/firestore/src/core/view.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -505,7 +505,7 @@ function compareChangeType(c1: ChangeType, c2: ChangeType): number {
505505
case ChangeType.Removed:
506506
return 0;
507507
default:
508-
return fail('Unknown ChangeType: ' + change);
508+
return fail('Unknown ChangeType', { change });
509509
}
510510
};
511511

0 commit comments

Comments
 (0)