Skip to content

Remove assert message (WIP) #2820

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 9 additions & 25 deletions packages/firestore/src/api/credentials.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,17 +116,14 @@ export class EmptyCredentialsProvider implements CredentialsProvider {
invalidateToken(): void {}

setChangeListener(changeListener: CredentialChangeListener): void {
assert(!this.changeListener, 'Can only call setChangeListener() once.');
assert(!this.changeListener);
this.changeListener = changeListener;
// Fire with initial user.
changeListener(User.UNAUTHENTICATED);
}

removeChangeListener(): void {
assert(
this.changeListener !== null,
'removeChangeListener() when no listener registered'
);
assert(this.changeListener !== null);
this.changeListener = null;
}
}
Expand Down Expand Up @@ -190,10 +187,7 @@ export class FirebaseCredentialsProvider implements CredentialsProvider {
}

getToken(): Promise<Token | null> {
assert(
this.tokenListener != null,
'getToken cannot be called after listener removed.'
);
assert(this.tokenListener != null);

// Take note of the current value of the tokenCounter so that this method
// can fail (with an ABORTED error) if there is a token change while the
Expand All @@ -217,10 +211,7 @@ export class FirebaseCredentialsProvider implements CredentialsProvider {
);
} else {
if (tokenData) {
assert(
typeof tokenData.accessToken === 'string',
'Invalid tokenData returned from getToken():' + tokenData
);
assert(typeof tokenData.accessToken === 'string');
return new OAuthToken(tokenData.accessToken, this.currentUser);
} else {
return null;
Expand All @@ -234,7 +225,7 @@ export class FirebaseCredentialsProvider implements CredentialsProvider {
}

setChangeListener(changeListener: CredentialChangeListener): void {
assert(!this.changeListener, 'Can only call setChangeListener() once.');
assert(!this.changeListener);
this.changeListener = changeListener;

// Fire the initial event
Expand All @@ -244,11 +235,8 @@ export class FirebaseCredentialsProvider implements CredentialsProvider {
}

removeChangeListener(): void {
assert(this.tokenListener != null, 'removeChangeListener() called twice');
assert(
this.changeListener !== null,
'removeChangeListener() called when no listener registered'
);
assert(this.tokenListener != null);
assert(this.changeListener !== null);

if (this.auth) {
this.auth.removeAuthTokenListener(this.tokenListener!);
Expand All @@ -263,10 +251,7 @@ export class FirebaseCredentialsProvider implements CredentialsProvider {
// to guarantee to get the actual user.
private getUser(): User {
const currentUid = this.auth && this.auth.getUid();
assert(
currentUid === null || typeof currentUid === 'string',
'Received invalid UID: ' + currentUid
);
assert(currentUid === null || typeof currentUid === 'string');
return new User(currentUid);
}
}
Expand Down Expand Up @@ -348,8 +333,7 @@ export function makeCredentialsProvider(
client !== null &&
client['auth'] &&
client['auth']['getAuthHeaderValueForFirstParty']
),
'unexpected gapi interface'
)
);
return new FirstPartyCredentialsProvider(
client,
Expand Down
36 changes: 11 additions & 25 deletions packages/firestore/src/api/database.ts
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
observer: PartialObserver<void>
): Unsubscribe {
const errHandler = (err: Error): void => {
throw fail('Uncaught Error in onSnapshotsInSync');
throw fail();
};
const asyncObserver = new AsyncObserver<void>({
next: () => {
Expand Down Expand Up @@ -514,9 +514,9 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
persistenceProvider: PersistenceProvider,
persistenceSettings: PersistenceSettings
): Promise<void> {
assert(!!this._settings.host, 'FirestoreSettings.host is not set');
assert(!!this._settings.host);

assert(!this._firestoreClient, 'configureClient() called multiple times');
assert(!this._firestoreClient);

const databaseInfo = this.makeDatabaseInfo();

Expand Down Expand Up @@ -712,7 +712,7 @@ export class Transaction implements firestore.Transaction {
.lookup([ref._key])
.then((docs: MaybeDocument[]) => {
if (!docs || docs.length !== 1) {
return fail('Mismatch in docs returned from document lookup.');
return fail();
}
const doc = docs[0];
if (doc instanceof NoDocument) {
Expand All @@ -734,9 +734,7 @@ export class Transaction implements firestore.Transaction {
ref._converter
);
} else {
throw fail(
`BatchGetDocumentsRequest returned unexpected document type: ${doc.constructor.name}`
);
throw fail();
}
});
}
Expand Down Expand Up @@ -1212,10 +1210,7 @@ export class DocumentReference<T = firestore.DocumentData>
const asyncObserver = new AsyncObserver<ViewSnapshot>({
next: snapshot => {
if (observer.next) {
assert(
snapshot.docs.size <= 1,
'Too many documents returned on a document query'
);
assert(snapshot.docs.size <= 1);
const doc = snapshot.docs.get(this._key);

observer.next(
Expand Down Expand Up @@ -1457,10 +1452,7 @@ export class QueryDocumentSnapshot<T = firestore.DocumentData>
implements firestore.QueryDocumentSnapshot<T> {
data(options?: SnapshotOptions): T {
const data = super.data(options);
assert(
data !== undefined,
'Document in a QueryDocumentSnapshot should exist'
);
assert(data !== undefined);
return data;
}
}
Expand Down Expand Up @@ -2539,14 +2531,8 @@ export function changesFromSnapshot<T>(
snapshot.mutatedKeys.has(change.doc.key),
converter
);
assert(
change.type === ChangeType.Added,
'Invalid event type for first snapshot'
);
assert(
!lastDoc || snapshot.query.docComparator(lastDoc, change.doc) < 0,
'Got added events in wrong order'
);
assert(change.type === ChangeType.Added);
assert(!lastDoc || snapshot.query.docComparator(lastDoc, change.doc) < 0);
lastDoc = change.doc;
return {
type: 'added' as firestore.DocumentChangeType,
Expand Down Expand Up @@ -2576,7 +2562,7 @@ export function changesFromSnapshot<T>(
let newIndex = -1;
if (change.type !== ChangeType.Added) {
oldIndex = indexTracker.indexOf(change.doc.key);
assert(oldIndex >= 0, 'Index for document not found');
assert(oldIndex >= 0);
indexTracker = indexTracker.delete(change.doc.key);
}
if (change.type !== ChangeType.Removed) {
Expand All @@ -2598,7 +2584,7 @@ function resultChangeType(type: ChangeType): firestore.DocumentChangeType {
case ChangeType.Removed:
return 'removed';
default:
return fail('Unknown change type: ' + type);
return fail();
}
}

Expand Down
21 changes: 6 additions & 15 deletions packages/firestore/src/api/user_data_reader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ function isWrite(dataSource: UserDataSource): boolean {
case UserDataSource.ArrayArgument:
return false;
default:
throw fail(`Unexpected case for UserDataSource: ${dataSource}`);
throw fail();
}
}

Expand Down Expand Up @@ -353,9 +353,7 @@ export class UserDataReader {
stringOrFieldPath
);
} else {
throw fail(
'Expected stringOrFieldPath to be a string or a FieldPath'
);
throw fail();
}

if (!context.contains(fieldPath)) {
Expand Down Expand Up @@ -494,11 +492,8 @@ export class UserDataReader {
FieldPath.EMPTY_PATH
);
const parsed = this.parseData(input, context);
assert(parsed != null, 'Parsed data should not be null.');
assert(
context.fieldTransforms.length === 0,
'Field transforms should have been disallowed.'
);
assert(parsed != null);
assert(context.fieldTransforms.length === 0);
return parsed;
}

Expand Down Expand Up @@ -633,11 +628,7 @@ export class UserDataReader {
// fieldMask so it gets deleted.
context.fieldMask.push(context.path);
} else if (context.dataSource === UserDataSource.Update) {
assert(
context.path.length > 0,
'FieldValue.delete() at the top level should have already' +
' been handled.'
);
assert(context.path.length > 0);
throw context.createError(
'FieldValue.delete() can only appear at the top level ' +
'of your update data'
Expand Down Expand Up @@ -684,7 +675,7 @@ export class UserDataReader {
new FieldTransform(context.path, numericIncrement)
);
} else {
fail('Unknown FieldValue type: ' + value);
fail();
}
}

Expand Down
7 changes: 2 additions & 5 deletions packages/firestore/src/api/user_data_writer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export class UserDataWriter<T = firestore.DocumentData> {
case TypeOrder.ObjectValue:
return this.convertObject(value.mapValue!);
default:
throw fail('Invalid value type: ' + JSON.stringify(value));
throw fail();
}
}

Expand Down Expand Up @@ -130,10 +130,7 @@ export class UserDataWriter<T = firestore.DocumentData> {

private convertReference(name: string): DocumentReference<T> {
const resourcePath = ResourcePath.fromString(name);
assert(
isValidResourceName(resourcePath),
'ReferenceValue is not valid ' + name
);
assert(isValidResourceName(resourcePath));
const databaseId = new DatabaseId(resourcePath.get(1), resourcePath.get(3));
const key = new DocumentKey(resourcePath.popFirst(5));

Expand Down
25 changes: 5 additions & 20 deletions packages/firestore/src/core/event_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,7 @@ export class EventManager implements SyncEngineListener {

// Run global snapshot listeners if a consistent snapshot has been emitted.
const raisedEvent = listener.applyOnlineStateChange(this.onlineState);
assert(
!raisedEvent,
"applyOnlineStateChange() shouldn't raise an event for brand-new listeners."
);
assert(!raisedEvent);

if (queryInfo.viewSnap) {
const raisedEvent = listener.onViewSnapshot(queryInfo.viewSnap);
Expand Down Expand Up @@ -226,10 +223,7 @@ export class QueryListener {
* indeed raised.
*/
onViewSnapshot(snap: ViewSnapshot): boolean {
assert(
snap.docChanges.length > 0 || snap.syncStateChanged,
'We got a new snapshot with no changes?'
);
assert(snap.docChanges.length > 0 || snap.syncStateChanged);

if (!this.options.includeMetadataChanges) {
// Remove the metadata only changes.
Expand Down Expand Up @@ -288,10 +282,7 @@ export class QueryListener {
snap: ViewSnapshot,
onlineState: OnlineState
): boolean {
assert(
!this.raisedInitialEvent,
'Determining whether to raise first event but already had first event'
);
assert(!this.raisedInitialEvent);

// Always raise the first event when we're synced
if (!snap.fromCache) {
Expand All @@ -304,10 +295,7 @@ export class QueryListener {
// Don't raise the event if we're online, aren't synced yet (checked
// above) and are waiting for a sync.
if (this.options.waitForSyncWhenOnline && maybeOnline) {
assert(
snap.fromCache,
'Waiting for sync, but snapshot is not from cache'
);
assert(snap.fromCache);
return false;
}

Expand Down Expand Up @@ -337,10 +325,7 @@ export class QueryListener {
}

private raiseInitialEvent(snap: ViewSnapshot): void {
assert(
!this.raisedInitialEvent,
'Trying to raise initial events for second time'
);
assert(!this.raisedInitialEvent);
snap = ViewSnapshot.fromInitialDocuments(
snap.query,
snap.docs,
Expand Down
Loading