Skip to content

Commit

Permalink
Change Error to FirestoreError (#3418)
Browse files Browse the repository at this point in the history
  • Loading branch information
dconeybe committed Sep 9, 2020
1 parent 3d9b5a5 commit a8ff3db
Show file tree
Hide file tree
Showing 11 changed files with 49 additions and 39 deletions.
6 changes: 6 additions & 0 deletions .changeset/thin-buses-fail.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@firebase/firestore': minor
'@firebase/firestore-types': minor
---

Use FirestoreError instead of Error in onSnapshot*() error callbacks.
16 changes: 8 additions & 8 deletions packages/firebase/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8215,7 +8215,7 @@ declare namespace firebase.firestore {
*/
onSnapshotsInSync(observer: {
next?: (value: void) => void;
error?: (error: Error) => void;
error?: (error: FirestoreError) => void;
complete?: () => void;
}): () => void;

Expand Down Expand Up @@ -8843,7 +8843,7 @@ declare namespace firebase.firestore {
options: SnapshotListenOptions,
observer: {
next?: (snapshot: DocumentSnapshot<T>) => void;
error?: (error: Error) => void;
error?: (error: FirestoreError) => void;
complete?: () => void;
}
): () => void;
Expand All @@ -8864,7 +8864,7 @@ declare namespace firebase.firestore {
*/
onSnapshot(
onNext: (snapshot: DocumentSnapshot<T>) => void,
onError?: (error: Error) => void,
onError?: (error: FirestoreError) => void,
onCompletion?: () => void
): () => void;
/**
Expand All @@ -8886,7 +8886,7 @@ declare namespace firebase.firestore {
onSnapshot(
options: SnapshotListenOptions,
onNext: (snapshot: DocumentSnapshot<T>) => void,
onError?: (error: Error) => void,
onError?: (error: FirestoreError) => void,
onCompletion?: () => void
): () => void;

Expand Down Expand Up @@ -9273,7 +9273,7 @@ declare namespace firebase.firestore {
*/
onSnapshot(observer: {
next?: (snapshot: QuerySnapshot<T>) => void;
error?: (error: Error) => void;
error?: (error: FirestoreError) => void;
complete?: () => void;
}): () => void;
/**
Expand All @@ -9294,7 +9294,7 @@ declare namespace firebase.firestore {
options: SnapshotListenOptions,
observer: {
next?: (snapshot: QuerySnapshot<T>) => void;
error?: (error: Error) => void;
error?: (error: FirestoreError) => void;
complete?: () => void;
}
): () => void;
Expand All @@ -9316,7 +9316,7 @@ declare namespace firebase.firestore {
*/
onSnapshot(
onNext: (snapshot: QuerySnapshot<T>) => void,
onError?: (error: Error) => void,
onError?: (error: FirestoreError) => void,
onCompletion?: () => void
): () => void;
/**
Expand All @@ -9339,7 +9339,7 @@ declare namespace firebase.firestore {
onSnapshot(
options: SnapshotListenOptions,
onNext: (snapshot: QuerySnapshot<T>) => void,
onError?: (error: Error) => void,
onError?: (error: FirestoreError) => void,
onCompletion?: () => void
): () => void;

Expand Down
16 changes: 8 additions & 8 deletions packages/firestore-types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ export class FirebaseFirestore {

onSnapshotsInSync(observer: {
next?: (value: void) => void;
error?: (error: Error) => void;
error?: (error: FirestoreError) => void;
complete?: () => void;
}): () => void;
onSnapshotsInSync(onSync: () => void): () => void;
Expand Down Expand Up @@ -235,19 +235,19 @@ export class DocumentReference<T = DocumentData> {
options: SnapshotListenOptions,
observer: {
next?: (snapshot: DocumentSnapshot<T>) => void;
error?: (error: Error) => void;
error?: (error: FirestoreError) => void;
complete?: () => void;
}
): () => void;
onSnapshot(
onNext: (snapshot: DocumentSnapshot<T>) => void,
onError?: (error: Error) => void,
onError?: (error: FirestoreError) => void,
onCompletion?: () => void
): () => void;
onSnapshot(
options: SnapshotListenOptions,
onNext: (snapshot: DocumentSnapshot<T>) => void,
onError?: (error: Error) => void,
onError?: (error: FirestoreError) => void,
onCompletion?: () => void
): () => void;

Expand Down Expand Up @@ -338,26 +338,26 @@ export class Query<T = DocumentData> {

onSnapshot(observer: {
next?: (snapshot: QuerySnapshot<T>) => void;
error?: (error: Error) => void;
error?: (error: FirestoreError) => void;
complete?: () => void;
}): () => void;
onSnapshot(
options: SnapshotListenOptions,
observer: {
next?: (snapshot: QuerySnapshot<T>) => void;
error?: (error: Error) => void;
error?: (error: FirestoreError) => void;
complete?: () => void;
}
): () => void;
onSnapshot(
onNext: (snapshot: QuerySnapshot<T>) => void,
onError?: (error: Error) => void,
onError?: (error: FirestoreError) => void,
onCompletion?: () => void
): () => void;
onSnapshot(
options: SnapshotListenOptions,
onNext: (snapshot: QuerySnapshot<T>) => void,
onError?: (error: Error) => void,
onError?: (error: FirestoreError) => void,
onCompletion?: () => void
): () => void;

Expand Down
16 changes: 8 additions & 8 deletions packages/firestore/exp/test/shim.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ export class FirebaseFirestore implements legacy.FirebaseFirestore {

onSnapshotsInSync(observer: {
next?: (value: void) => void;
error?: (error: Error) => void;
error?: (error: legacy.FirestoreError) => void;
complete?: () => void;
}): () => void;
onSnapshotsInSync(onSync: () => void): () => void;
Expand Down Expand Up @@ -370,19 +370,19 @@ export class DocumentReference<T = legacy.DocumentData>
options: legacy.SnapshotListenOptions,
observer: {
next?: (snapshot: DocumentSnapshot<T>) => void;
error?: (error: Error) => void;
error?: (error: legacy.FirestoreError) => void;
complete?: () => void;
}
): () => void;
onSnapshot(
onNext: (snapshot: DocumentSnapshot<T>) => void,
onError?: (error: Error) => void,
onError?: (error: legacy.FirestoreError) => void,
onCompletion?: () => void
): () => void;
onSnapshot(
options: legacy.SnapshotListenOptions,
onNext: (snapshot: DocumentSnapshot<T>) => void,
onError?: (error: Error) => void,
onError?: (error: legacy.FirestoreError) => void,
onCompletion?: () => void
): () => void;
onSnapshot(...args: any): () => void {
Expand Down Expand Up @@ -530,26 +530,26 @@ export class Query<T = legacy.DocumentData> implements legacy.Query<T> {

onSnapshot(observer: {
next?: (snapshot: QuerySnapshot<T>) => void;
error?: (error: Error) => void;
error?: (error: legacy.FirestoreError) => void;
complete?: () => void;
}): () => void;
onSnapshot(
options: legacy.SnapshotListenOptions,
observer: {
next?: (snapshot: QuerySnapshot<T>) => void;
error?: (error: Error) => void;
error?: (error: legacy.FirestoreError) => void;
complete?: () => void;
}
): () => void;
onSnapshot(
onNext: (snapshot: QuerySnapshot<T>) => void,
onError?: (error: Error) => void,
onError?: (error: legacy.FirestoreError) => void,
onCompletion?: () => void
): () => void;
onSnapshot(
options: legacy.SnapshotListenOptions,
onNext: (snapshot: QuerySnapshot<T>) => void,
onError?: (error: Error) => void,
onError?: (error: legacy.FirestoreError) => void,
onCompletion?: () => void
): () => void;
onSnapshot(...args: any): () => void {
Expand Down
3 changes: 2 additions & 1 deletion packages/firestore/src/api/observer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,13 @@
*/

import { JsonObject } from '../model/object_value';
import { FirestoreError } from '../util/error';

/**
* Observer/Subscribe interfaces.
*/
export type NextFn<T> = (value: T) => void;
export type ErrorFn = (error: Error) => void;
export type ErrorFn = (error: FirestoreError) => void;
export type CompleteFn = () => void;

// Allow for any of the Observer methods to be undefined.
Expand Down
4 changes: 2 additions & 2 deletions packages/firestore/src/api/user_data_reader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ export class ParseContext {
return this.contextWith({ path: undefined, arrayElement: true });
}

createError(reason: string): Error {
createError(reason: string): FirestoreError {
return createError(
reason,
this.settings.methodName,
Expand Down Expand Up @@ -832,7 +832,7 @@ function createError(
hasConverter: boolean,
path?: FieldPath,
targetDoc?: DocumentKey
): Error {
): FirestoreError {
const hasPath = path && !path.isEmpty();
const hasDocument = targetDoc !== undefined;
let message = `Function ${methodName}() called with invalid data`;
Expand Down
7 changes: 4 additions & 3 deletions packages/firestore/src/core/event_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/

import { debugAssert, debugCast } from '../util/assert';
import { FirestoreError } from '../util/error';
import { EventHandler } from '../util/misc';
import { ObjectMap } from '../util/obj_map';
import { canonifyQuery, Query, queryEquals, stringifyQuery } from './query';
Expand All @@ -37,7 +38,7 @@ class QueryListenersInfo {
*/
export interface Observer<T> {
next: EventHandler<T>;
error: EventHandler<Error>;
error: EventHandler<FirestoreError>;
}

/**
Expand Down Expand Up @@ -175,7 +176,7 @@ export function eventManagerOnWatchChange(
export function eventManagerOnWatchError(
eventManager: EventManager,
query: Query,
error: Error
error: FirestoreError
): void {
const eventManagerImpl = debugCast(eventManager, EventManagerImpl);

Expand Down Expand Up @@ -323,7 +324,7 @@ export class QueryListener {
return raisedEvent;
}

onError(error: Error): void {
onError(error: FirestoreError): void {
this.queryObserver.error(error);
}

Expand Down
6 changes: 3 additions & 3 deletions packages/firestore/src/core/sync_engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ export interface SyncEngineListener {
onWatchChange(snapshots: ViewSnapshot[]): void;

/** Handles the failure of a query. */
onWatchError(query: Query, error: Error): void;
onWatchError(query: Query, error: FirestoreError): void;

/** Handles a change in online state. */
onOnlineStateChange(onlineState: OnlineState): void;
Expand Down Expand Up @@ -817,7 +817,7 @@ function addMutationCallback(
export function processUserCallback(
syncEngine: SyncEngine,
batchId: BatchId,
error: Error | null
error: FirestoreError | null
): void {
const syncEngineImpl = debugCast(syncEngine, SyncEngineImpl);
let newCallbacks =
Expand Down Expand Up @@ -848,7 +848,7 @@ export function processUserCallback(
function removeAndCleanupTarget(
syncEngineImpl: SyncEngineImpl,
targetId: number,
error: Error | null = null
error: FirestoreError | null = null
): void {
syncEngineImpl.sharedClientState.removeLocalQueryTarget(targetId);

Expand Down
3 changes: 2 additions & 1 deletion packages/firestore/src/util/async_observer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import { Observer } from '../core/event_manager';
import { EventHandler } from './misc';
import { FirestoreError } from './error';

/*
* A wrapper implementation of Observer<T> that will dispatch events
Expand All @@ -38,7 +39,7 @@ export class AsyncObserver<T> implements Observer<T> {
}
}

error(error: Error): void {
error(error: FirestoreError): void {
if (this.observer.error) {
this.scheduleEvent(this.observer.error, error);
} else {
Expand Down
2 changes: 1 addition & 1 deletion packages/firestore/src/util/async_queue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ export class AsyncQueue {
private delayedOperations: Array<DelayedOperation<unknown>> = [];

// visible for testing
failure: Error | null = null;
failure: FirestoreError | null = null;

// Flag set while there's an outstanding AsyncQueue operation, used for
// assertion sanity-checks.
Expand Down
9 changes: 5 additions & 4 deletions packages/firestore/test/unit/core/event_manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import { View } from '../../../src/core/view';
import { ChangeType, ViewSnapshot } from '../../../src/core/view_snapshot';
import { documentKeySet } from '../../../src/model/collections';
import { DocumentSet } from '../../../src/model/document_set';
import { Code, FirestoreError } from '../../../src/util/error';
import { addEqualityMatcher } from '../../util/equality_matcher';
import {
ackTarget,
Expand Down Expand Up @@ -164,7 +165,7 @@ describe('QueryListener', () => {
function queryListener(
query: Query,
events?: ViewSnapshot[],
errors?: Error[],
errors?: FirestoreError[],
options?: ListenOptions
): QueryListener {
return new QueryListener(
Expand All @@ -175,7 +176,7 @@ describe('QueryListener', () => {
events.push(snap);
}
},
error: (error: Error) => {
error: (error: FirestoreError) => {
if (errors !== undefined) {
errors.push(error);
}
Expand Down Expand Up @@ -228,11 +229,11 @@ describe('QueryListener', () => {
});

it('raises error event', () => {
const events: Error[] = [];
const events: FirestoreError[] = [];
const query1 = query('rooms/Eros');

const listener = queryListener(query1, [], events);
const error = new Error('bad');
const error = new FirestoreError(Code.UNKNOWN, 'bad');

listener.onError(error);
expect(events[0]).to.deep.equal(error);
Expand Down

0 comments on commit a8ff3db

Please sign in to comment.