Skip to content

feat: throw dedicated errors instead of generic ones #39

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 1 commit into from
Sep 1, 2021
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
feat: throw dedicated errors instead of generic ones
  • Loading branch information
hellivan committed Sep 1, 2021
commit 10f39a8cf1a498a0bb1f5e31ccf4a006406f1315
3 changes: 2 additions & 1 deletion src/data-structures/limited-observable-queue.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { QueueError } from '../errors';
import { ObservableQueue } from './observable-queue';

export class LimitedObservableQueue<T> extends ObservableQueue<T> {
Expand Down Expand Up @@ -25,7 +26,7 @@ export class LimitedObservableQueue<T> extends ObservableQueue<T> {
}

public enqueue(item: T): void {
if (this.full) throw new Error('Queue full! Cannot enqueue any more items!');
if (this.full) throw new QueueError('Queue full! Cannot enqueue any more items!');
super.enqueue(item);
}
}
8 changes: 8 additions & 0 deletions src/errors/execution-timeout-error.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
export class ExecutionTimeoutError extends Error {
public readonly code = 'NTSU_EXEC_TIMEOUT';

constructor(message: string) {
super(message);
Object.setPrototypeOf(this, ExecutionTimeoutError.prototype);
}
}
4 changes: 4 additions & 0 deletions src/errors/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export { ExecutionTimeoutError } from './execution-timeout-error';
export { QueueError } from './queue-error';
export { WorkerError } from './worker-error';
export { WorkerPoolError } from './worker-pool-error';
8 changes: 8 additions & 0 deletions src/errors/queue-error.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
export class QueueError extends Error {
public readonly code = 'NTSU_QUEUE_ERROR';

constructor(message: string) {
super(message);
Object.setPrototypeOf(this, QueueError.prototype);
}
}
8 changes: 8 additions & 0 deletions src/errors/worker-error.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
export class WorkerError extends Error {
public readonly code = 'NTSU_WORKER_ERROR';

constructor(message: string) {
super(message);
Object.setPrototypeOf(this, WorkerError.prototype);
}
}
8 changes: 8 additions & 0 deletions src/errors/worker-pool-error.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
export class WorkerPoolError extends Error {
public readonly code = 'NTSU_WORKER_POOL_ERROR';

constructor(message: string) {
super(message);
Object.setPrototypeOf(this, WorkerPoolError.prototype);
}
}
12 changes: 12 additions & 0 deletions src/index.spec.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
import {
ExecutionTimeoutError,
DynamicWorkerPool,
DynamicWorkerPoolWorker,
QueueError,
WorkerError,
WorkerFactory,
WorkerPool,
WorkerPoolError,
WorkerThreadTask,
WorkerThreadWorker,
QdScheduler
Expand All @@ -15,6 +19,14 @@ describe('exports', () => {
expect(WorkerThreadWorker).toBeDefined();

expect(QdScheduler).toBeDefined();

expect(ExecutionTimeoutError).toBeDefined();

expect(QueueError).toBeDefined();

expect(WorkerError).toBeDefined();

expect(WorkerPoolError).toBeDefined();
});

test('interfaces must be exported', () => {
Expand Down
1 change: 1 addition & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export { ExecutionTimeoutError, QueueError, WorkerError, WorkerPoolError } from './errors';
export { WorkerThreadTask, WorkerThreadWorker } from './workers';
export { WorkerFactory } from './worker-factory';
export { DynamicWorkerPool, DynamicWorkerPoolWorker, WorkerPool } from './worker-pool';
Expand Down
7 changes: 4 additions & 3 deletions src/worker-pool/dynamic-worker-pool.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { BehaviorSubject, distinctUntilChanged, filter, firstValueFrom, Observable, Subject } from 'rxjs';

import { ObservableQueue } from '../data-structures';
import { WorkerPoolError } from '../errors';
import { WorkerFactory } from '../worker-factory';
import { AbstractWorkerPool, AbstractWorkerPoolWorker } from './abstract-worker-pool';
import { IdleWorkerDescription } from './idle-worker-description';
Expand Down Expand Up @@ -78,7 +79,7 @@ export class DynamicWorkerPool<

protected aquireWorker(): TWorker {
if (this.stopped) {
throw new Error('Cannot aquire worker from stopped worker pool!');
throw new WorkerPoolError('Cannot aquire worker from stopped worker pool!');
} else if (this.idleWorkersQueue.size > 0) {
const workerDescription = this.idleWorkersQueue.dequeue();
workerDescription.cancelIdleTimeout();
Expand All @@ -93,7 +94,7 @@ export class DynamicWorkerPool<
this.updateAvailableWorkers();
return worker;
} else {
throw new Error(`No free workers available in worker pool of max-size ${this.maxSize}`);
throw new WorkerPoolError(`No free workers available in worker pool of max-size ${this.maxSize}`);
}
}

Expand Down Expand Up @@ -133,7 +134,7 @@ export class DynamicWorkerPool<
try {
await worker.dispose();
} catch (err) {
this.errorSubject.next(new Error(`Error while disposing worker-pool worker: ${err.message}`));
this.errorSubject.next(new WorkerPoolError(`Error while disposing worker-pool worker: ${err.message}`));
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/workers/worker-thread-worker.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ describe('WorkerThreadWorker', () => {

const emittedError = await errorEmitterPromise;
expect(emittedError).toMatchObject({
message: 'Worker terminated gracefully. This hould not happen in a WorkerThreadWorker!'
message: 'Worker terminated gracefully. This should not happen in a WorkerThreadWorker!'
});
});

Expand Down
21 changes: 13 additions & 8 deletions src/workers/worker-thread-worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { Subject, Observable } from 'rxjs';

import { EventRegistrationList } from './event-registration-list';
import { TimerEventRegistration } from './timer-event-registration';
import { ExecutionTimeoutError, WorkerError } from '../errors';

export interface WorkerThreadTask<TData> {
readonly timeout: number;
Expand All @@ -27,7 +28,7 @@ export class WorkerThreadWorker<TData, TResult, TTask extends WorkerThreadTask<T
}

public async executeTask({ timeout, data }: TTask): Promise<TResult> {
if (this.working) throw new Error('Worker busy. Cannot execute mutiple tasks in parallel!');
if (this.working) throw new WorkerError('Worker busy. Cannot execute mutiple tasks in parallel!');
this.working = true;

try {
Expand All @@ -48,15 +49,17 @@ export class WorkerThreadWorker<TData, TResult, TTask extends WorkerThreadTask<T

const onExit = (code: number): void => {
eventRegistrationsList.unregisterAll();
reject(new Error(`Worker terminated with code ${code}`));
reject(new WorkerError(`Worker terminated with code ${code}`));
};

eventRegistrationsList.push(
new TimerEventRegistration(() => {
eventRegistrationsList.unregisterAll();
// NOTE: terminateWorker will never reject. Hence we can do not have to catch rejection here
this.terminateWorker();
const timeoutError = new Error(`Worker execution timed out after ${timeout} ms`);
const timeoutError = new ExecutionTimeoutError(
`Worker execution timed out after ${timeout} ms`
);
this.errorsSubject.next(timeoutError);
reject(timeoutError);
}, timeout)
Expand Down Expand Up @@ -99,15 +102,17 @@ export class WorkerThreadWorker<TData, TResult, TTask extends WorkerThreadTask<T

const terminationPromise = this.workerInstance.worker
.terminate()
.catch((err) => this.errorsSubject.next(new Error(`Error while terminating worker: ${err.message}`)));
.catch((err) =>
this.errorsSubject.next(new WorkerError(`Error while terminating worker: ${err.message}`))
);

this.workerInstance = undefined;
await terminationPromise;
}
}

private getWorker(): Worker {
if (this.disposed) throw new Error('Cannot get worker in a disposed WorkerThreadWorker!');
if (this.disposed) throw new WorkerError('Cannot get worker in a disposed WorkerThreadWorker!');

if (this.workerInstance == null) {
const newWorker = new Worker(this.workerScriptPath);
Expand All @@ -116,17 +121,17 @@ export class WorkerThreadWorker<TData, TResult, TTask extends WorkerThreadTask<T
const onError = (err: Error): void => {
eventRegistrationsList.unregisterAll();
this.workerInstance = undefined;
this.errorsSubject.next(new Error(`Worker failed with error: ${err.message}`));
this.errorsSubject.next(new WorkerError(`Worker failed with error: ${err.message}`));
};

const onExit = (code: number): void => {
eventRegistrationsList.unregisterAll();
this.workerInstance = undefined;
if (code !== 0) {
this.errorsSubject.next(new Error(`Worker terminated with exit code ${code}!`));
this.errorsSubject.next(new WorkerError(`Worker terminated with exit code ${code}!`));
} else {
this.errorsSubject.next(
new Error(`Worker terminated gracefully. This hould not happen in a WorkerThreadWorker!`)
new WorkerError(`Worker terminated gracefully. This should not happen in a WorkerThreadWorker!`)
);
}
};
Expand Down