Skip to content

Commit

Permalink
feat: add new error handling strategy
Browse files Browse the repository at this point in the history
Resolves #46
  • Loading branch information
lfilho committed Jul 26, 2020
1 parent 147a03b commit 0f210a4
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 18 deletions.
4 changes: 2 additions & 2 deletions __tests__/src/shared/metric_service.spec.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import MetricService from '../../../src/shared/metric_service.js';
import { InvalidMetricTypeError } from '../../../src/shared/model/error.js';

describe('Metric Service', () => {
it('should throw if trying to emit an non-Metric', async () => {
await expect(async () => {
await MetricService.emit('my-made-up-metric');
}).rejects.toThrow(/Invalid metric/);
//TODO https://github.com/lfilho/sample-webextension/issues/46
}).rejects.toThrow(InvalidMetricTypeError);
});
});
10 changes: 6 additions & 4 deletions __tests__/src/shared/model/metric.spec.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
import Metric from '../../../../src/shared/model/metric.js';
import {
InvalidMetricDimensionError,
MetricInitializationError,
} from '../../../../src/shared/model/error.js';

describe('Metric', () => {
it('should throw with an inexisting dimension', () => {
expect(() => {
new Metric('my-made-up-dimension', 1);
}).toThrow(/Invalid dimension/);
//TODO https://github.com/lfilho/sample-webextension/issues/46
}).toThrow(InvalidMetricDimensionError);
});

it('accepts a known dimension', () => {
Expand All @@ -17,7 +20,6 @@ describe('Metric', () => {
it('requires both dimension and value args', () => {
expect(() => {
new Metric('one-arg');
}).toThrow(/Metric constructor needs both a dimension and value for it/);
//TODO https://github.com/lfilho/sample-webextension/issues/46
}).toThrow(MetricInitializationError);
});
});
5 changes: 3 additions & 2 deletions src/lib/model/list.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { ListTypeError } from '../../shared/model/error.js';

export default class List {
/*
* Using a Set and mainly proxying the methods to their native ones.
Expand All @@ -7,8 +9,7 @@ export default class List {

constructor(type) {
if (!type) {
throw new Error('List constructor needs a type');
//TODO https://github.com/lfilho/sample-webextension/issues/46
throw new ListTypeError();
}
this.list = new Set();
this.type = type;
Expand Down
10 changes: 6 additions & 4 deletions src/shared/metric_service.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
import Metric from './model/metric.js';
import fetch from './util/fake_fetch.js';
import Logger from './util/logger.js';
import {
InvalidMetricTypeError,
ErrorSendingMetric,
} from '../shared/model/error.js';

export default class MetricService {
static async emit(metric) {
if (!(metric instanceof Metric)) {
throw new Error('Invalid metric.');
throw new InvalidMetricTypeError();
}

const metricPayload = getMetricPayload(metric);
Expand All @@ -20,9 +24,7 @@ export default class MetricService {
Logger.info(metricString);
})
.catch((error) => {
Logger.error(
`Error while sending metric. Did you pay your internet bill? ${error}`
);
Logger.error(new ErrorSendingMetric(error));
});
}
}
Expand Down
43 changes: 43 additions & 0 deletions src/shared/model/error.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* @see
* /docs/development/ARCHITECTURE.md#error-handling
*/

// Fake translation function, here as an example only
const t = (string) => string;

// Not supposed to be used directly, hence why not exporting it
class MyProjectError extends Error {
constructor(message) {
/*
* By calling `super` without args here,
* we force ourselves to create a unique message in each error subclass instead
*/
super();

this.name = this.constructor.name; // subclass' name

// The argument passed will instead be used as additional debugging message
this.debugMessage = message;
}
}

export class InvalidMetricTypeError extends MyProjectError {
message = t('Invalid metric. Metric must be an instance of Metric');
}

export class ErrorSendingMetric extends MyProjectError {
message = t('Error while sending metric. Did you pay your internet bill?');
}

export class MetricInitializationError extends MyProjectError {
message = t('Metric constructor needs both a dimension and value for it');
}

export class InvalidMetricDimensionError extends MyProjectError {
message = t('Invalid metric dimension. See `Metric.dimensions` for values.');
}

export class ListTypeError extends MyProjectError {
message = t('List constructor needs a type. See `List.types` for values.');
}
14 changes: 8 additions & 6 deletions src/shared/model/metric.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
import {
MetricInitializationError,
InvalidMetricDimensionError,
} from '../../shared/model/error.js';

const DIMENSIONS = Object.freeze({
REQUEST_BLOCKED: 'REQUEST_BLOCKED',
});
Expand All @@ -8,15 +13,12 @@ export default class Metric {
*/
constructor(dimension, value) {
if (!dimension || !value) {
//TODO https://github.com/lfilho/sample-webextension/issues/46
throw new Error(
'Metric constructor needs both a dimension and value for it'
);
throw new MetricInitializationError();
}

const possibleDimensions = Object.keys(DIMENSIONS);
if (!possibleDimensions.includes(dimension)) {
//TODO https://github.com/lfilho/sample-webextension/issues/46
throw new Error(`Invalid dimension. Valid ones: ${possibleDimensions}`);
throw new InvalidMetricDimensionError();
}

this.dimension = dimension;
Expand Down

0 comments on commit 0f210a4

Please sign in to comment.