Skip to content

Change Error to FirestoreError #3418

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

Merged
merged 13 commits into from
Sep 9, 2020
Merged
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
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