Skip to content

Commit

Permalink
Add types to ErrorFactory message parameters (#1720)
Browse files Browse the repository at this point in the history
* Add types to ErrorFactory message parameters

* Add ErrorParams to packages that use ErrorFactory
  • Loading branch information
mmermerkaya authored May 23, 2019
1 parent 6dc1533 commit cee7ad8
Show file tree
Hide file tree
Showing 10 changed files with 75 additions and 27 deletions.
10 changes: 6 additions & 4 deletions packages/app/src/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,10 @@ const ERRORS: ErrorMap<AppError> = {
'Firebase App instance.'
};

const appErrors = new ErrorFactory<AppError>('app', 'Firebase', ERRORS);
type ErrorParams = { [key in AppError]: { name: string } };

export function error(code: AppError, args?: { [name: string]: any }) {
throw appErrors.create(code, args);
}
export const ERROR_FACTORY = new ErrorFactory<AppError, ErrorParams>(
'app',
'Firebase',
ERRORS
);
4 changes: 2 additions & 2 deletions packages/app/src/firebaseApp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import {
FirebaseAppInternals
} from '@firebase/app-types/private';
import { deepCopy, deepExtend } from '@firebase/util';
import { error, AppError } from './errors';
import { AppError, ERROR_FACTORY } from './errors';
import { DEFAULT_ENTRY_NAME } from './constants';

interface ServicesCache {
Expand Down Expand Up @@ -202,7 +202,7 @@ export class FirebaseAppImpl implements FirebaseApp {
*/
private checkDestroyed_(): void {
if (this.isDeleted_) {
error(AppError.APP_DELETED, { name: this.name_ });
throw ERROR_FACTORY.create(AppError.APP_DELETED, { name: this.name_ });
}
}
}
Expand Down
14 changes: 8 additions & 6 deletions packages/app/src/firebaseNamespaceCore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import {
} from '@firebase/app-types/private';
import { deepExtend, patchProperty } from '@firebase/util';
import { FirebaseAppImpl } from './firebaseApp';
import { error, AppError } from './errors';
import { ERROR_FACTORY, AppError } from './errors';
import { FirebaseAppLiteImpl } from './lite/firebaseAppLite';
import { DEFAULT_ENTRY_NAME } from './constants';
import { version } from '../../firebase/package.json';
Expand Down Expand Up @@ -105,7 +105,7 @@ export function createFirebaseNamespaceCore(
function app(name?: string): FirebaseApp {
name = name || DEFAULT_ENTRY_NAME;
if (!contains(apps, name)) {
error(AppError.NO_APP, { name: name });
throw ERROR_FACTORY.create(AppError.NO_APP, { name: name });
}
return apps[name];
}
Expand Down Expand Up @@ -134,11 +134,11 @@ export function createFirebaseNamespaceCore(
const { name } = config;

if (typeof name !== 'string' || !name) {
error(AppError.BAD_APP_NAME, { name: String(name) });
throw ERROR_FACTORY.create(AppError.BAD_APP_NAME, { name: String(name) });
}

if (contains(apps, name)) {
error(AppError.DUPLICATE_APP, { name: name });
throw ERROR_FACTORY.create(AppError.DUPLICATE_APP, { name: name });
}

const app = new firebaseAppImpl(
Expand Down Expand Up @@ -177,7 +177,7 @@ export function createFirebaseNamespaceCore(
): FirebaseServiceNamespace<FirebaseService> {
// Cannot re-register a service that already exists
if (factories[name]) {
error(AppError.DUPLICATE_SERVICE, { name: name });
throw ERROR_FACTORY.create(AppError.DUPLICATE_SERVICE, { name: name });
}

// Capture the service factory for later service instantiation
Expand All @@ -198,7 +198,9 @@ export function createFirebaseNamespaceCore(
if (typeof appArg[name] !== 'function') {
// Invalid argument.
// This happens in the following case: firebase.storage('gs:/')
error(AppError.INVALID_APP_ARGUMENT, { name: name });
throw ERROR_FACTORY.create(AppError.INVALID_APP_ARGUMENT, {
name: name
});
}

// Forward service instance lookup to the FirebaseApp.
Expand Down
4 changes: 2 additions & 2 deletions packages/app/src/lite/firebaseAppLite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {
FirebaseService
} from '@firebase/app-types/private';
import { deepCopy, deepExtend } from '@firebase/util';
import { error, AppError } from '../errors';
import { ERROR_FACTORY, AppError } from '../errors';
import { DEFAULT_ENTRY_NAME } from '../constants';

interface ServicesCache {
Expand Down Expand Up @@ -168,7 +168,7 @@ export class FirebaseAppLiteImpl implements FirebaseApp {
*/
private checkDestroyed_(): void {
if (this.isDeleted_) {
error(AppError.APP_DELETED, { name: this.name_ });
throw ERROR_FACTORY.create(AppError.APP_DELETED, { name: this.name_ });
}
}
}
8 changes: 7 additions & 1 deletion packages/installations/src/util/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,13 @@ const ERROR_DESCRIPTION_MAP: { readonly [key in ErrorCode]: string } = {
"Can't delete installation while there is a pending registration request."
};

export const ERROR_FACTORY = new ErrorFactory(
interface ErrorParams {
[ErrorCode.REQUEST_FAILED]: {
requestName: string;
} & ServerErrorData;
}

export const ERROR_FACTORY = new ErrorFactory<ErrorCode, ErrorParams>(
SERVICE,
SERVICE_NAME,
ERROR_DESCRIPTION_MAP
Expand Down
10 changes: 9 additions & 1 deletion packages/messaging/src/models/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,15 @@ export const ERROR_MAP: ErrorMap<ErrorCode> = {
'The public VAPID key did not equal 65 bytes when decrypted.'
};

export const errorFactory = new ErrorFactory(
interface ErrorParams {
[ErrorCode.FAILED_DEFAULT_REGISTRATION]: { browserErrorMessage: string };
[ErrorCode.TOKEN_SUBSCRIBE_FAILED]: { errorInfo: string };
[ErrorCode.TOKEN_UNSUBSCRIBE_FAILED]: { errorInfo: string };
[ErrorCode.TOKEN_UPDATE_FAILED]: { errorInfo: string };
[ErrorCode.UNABLE_TO_RESUBSCRIBE]: { errorInfo: string };
}

export const errorFactory = new ErrorFactory<ErrorCode, ErrorParams>(
'messaging',
'Messaging',
ERROR_MAP
Expand Down
12 changes: 9 additions & 3 deletions packages/messaging/src/models/iid-model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,9 @@ export class IidModel {

responseData = await response.json();
} catch (err) {
throw errorFactory.create(ErrorCode.TOKEN_SUBSCRIBE_FAILED);
throw errorFactory.create(ErrorCode.TOKEN_SUBSCRIBE_FAILED, {
errorInfo: err
});
}

if (responseData.error) {
Expand Down Expand Up @@ -144,7 +146,9 @@ export class IidModel {
);
responseData = await response.json();
} catch (err) {
throw errorFactory.create(ErrorCode.TOKEN_UPDATE_FAILED);
throw errorFactory.create(ErrorCode.TOKEN_UPDATE_FAILED, {
errorInfo: err
});
}

if (responseData.error) {
Expand Down Expand Up @@ -196,7 +200,9 @@ export class IidModel {
});
}
} catch (err) {
throw errorFactory.create(ErrorCode.TOKEN_UNSUBSCRIBE_FAILED);
throw errorFactory.create(ErrorCode.TOKEN_UNSUBSCRIBE_FAILED, {
errorInfo: err
});
}
}
}
7 changes: 6 additions & 1 deletion packages/performance/src/utils/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,12 @@ const ERROR_DESCRIPTION_MAP: { readonly [key in ErrorCode]: string } = {
[ErrorCode.RC_NOT_OK]: 'RC response is not ok'
};

export const ERROR_FACTORY = new ErrorFactory(
interface ErrorParams {
[ErrorCode.TRACE_STARTED_BEFORE]: { traceName: string };
[ErrorCode.TRACE_STOPPED_BEFORE]: { traceName: string };
}

export const ERROR_FACTORY = new ErrorFactory<ErrorCode, ErrorParams>(
SERVICE,
SERVICE_NAME,
ERROR_DESCRIPTION_MAP
Expand Down
17 changes: 12 additions & 5 deletions packages/util/src/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,18 +103,25 @@ export class FirebaseError extends Error {
}
}

export class ErrorFactory<ErrorCode extends string> {
export class ErrorFactory<
ErrorCode extends string,
ErrorParams extends { readonly [K in ErrorCode]?: ErrorData } = {}
> {
constructor(
private readonly service: string,
private readonly serviceName: string,
private readonly errors: ErrorMap<ErrorCode>
) {}

create(code: ErrorCode, data: ErrorData = {}): FirebaseError {
create<K extends ErrorCode>(
code: K,
...data: K extends keyof ErrorParams ? [ErrorParams[K]] : []
): FirebaseError {
const customData = data[0] || {};
const fullCode = `${this.service}/${code}`;
const template = this.errors[code];

const message = template ? replaceTemplate(template, data) : 'Error';
const message = template ? replaceTemplate(template, customData) : 'Error';
// Service Name: Error message (service/code).
const fullMessage = `${this.serviceName}: ${message} (${fullCode}).`;

Expand All @@ -123,14 +130,14 @@ export class ErrorFactory<ErrorCode extends string> {
// Keys with an underscore at the end of their name are not included in
// error.data for some reason.
// TODO: Replace with Object.entries when lib is updated to es2017.
for (const key of Object.keys(data)) {
for (const key of Object.keys(customData)) {
if (key.slice(-1) !== '_') {
if (key in error) {
console.warn(
`Overwriting FirebaseError base field "${key}" can cause unexpected behavior.`
);
}
error[key] = data[key];
error[key] = customData[key];
}
}

Expand Down
16 changes: 14 additions & 2 deletions packages/util/test/errors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,17 @@ const ERROR_MAP: ErrorMap<ErrorCode> = {
'I decided to use {$code} to represent the error code from my server.'
};

const ERROR_FACTORY = new ErrorFactory<ErrorCode>('fake', 'Fake', ERROR_MAP);
interface ErrorParams {
'file-not-found': { file: string };
'anon-replace': { repl_: string };
'overwrite-field': { code: string };
}

const ERROR_FACTORY = new ErrorFactory<ErrorCode, ErrorParams>(
'fake',
'Fake',
ERROR_MAP
);

describe('FirebaseError', () => {
it('creates an Error', () => {
Expand Down Expand Up @@ -68,7 +78,9 @@ describe('FirebaseError', () => {
});

it('uses the key in the template if the replacement is missing', () => {
const e = ERROR_FACTORY.create('file-not-found', { fileX: 'foo.txt' });
const e = ERROR_FACTORY.create('file-not-found', {
fileX: 'foo.txt'
} as any);
assert.equal(e.code, 'fake/file-not-found');
assert.equal(
e.message,
Expand Down

0 comments on commit cee7ad8

Please sign in to comment.