Skip to content

Commit

Permalink
Unify error throwing methods (#878)
Browse files Browse the repository at this point in the history 
This commit unifies error throwing methods and removes throw error in
the logger.

Previously, implementation of the logger includes both logging and error
throwing, which can lead to confusion and type problems. By separating
these concerns, we can ensure that the logger is only responsible for
logging messages, while errors are thrown explicitly using `YorkieError`.
  • Loading branch information
@gwbaik9717 @hackerwins
gwbaik9717 authored and hackerwins committed Aug 7, 2024
1 parent 3e6d0b2 commit 267878f
Show file tree
Hide file tree
Showing 30 changed files with 443 additions and 183 deletions.
18 changes: 12 additions & 6 deletions src/api/converter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,10 @@ function toValueType(valueType: PrimitiveType): PbValueType {
case PrimitiveType.Date:
return PbValueType.DATE;
default:
throw new YorkieError(Code.Unsupported, `unsupported type: ${valueType}`);
throw new YorkieError(
Code.ErrInvalidType,
`unsupported type: ${valueType}`,
);
}
}

Expand All @@ -216,7 +219,10 @@ function toCounterType(valueType: CounterType): PbValueType {
case CounterType.LongCnt:
return PbValueType.LONG_CNT;
default:
throw new YorkieError(Code.Unsupported, `unsupported type: ${valueType}`);
throw new YorkieError(
Code.ErrInvalidType,
`unsupported type: ${valueType}`,
);
}
}

Expand Down Expand Up @@ -859,7 +865,7 @@ function fromPresenceChange<P extends Indexable>(
};
}

throw new YorkieError(Code.Unsupported, `unsupported type: ${type}`);
throw new YorkieError(Code.ErrInvalidType, `unsupported type: ${type}`);
}

/**
Expand Down Expand Up @@ -1476,7 +1482,7 @@ function bytesToSnapshot<P extends Indexable>(
*/
function bytesToObject(bytes?: Uint8Array): CRDTObject {
if (!bytes) {
throw new Error('bytes is empty');
throw new YorkieError(Code.ErrInvalidArgument, 'bytes is empty');
}

const pbElement = PbJSONElement.fromBinary(bytes);
Expand All @@ -1495,7 +1501,7 @@ function objectToBytes(obj: CRDTObject): Uint8Array {
*/
function bytesToArray(bytes?: Uint8Array): CRDTArray {
if (!bytes) {
throw new Error('bytes is empty');
throw new YorkieError(Code.ErrInvalidArgument, 'bytes is empty');
}

const pbElement = PbJSONElement.fromBinary(bytes);
Expand All @@ -1514,7 +1520,7 @@ function arrayToBytes(array: CRDTArray): Uint8Array {
*/
function bytesToTree(bytes?: Uint8Array): CRDTTree {
if (!bytes) {
throw new Error('bytes is empty');
throw new YorkieError(Code.ErrInvalidArgument, 'bytes is empty');
}

const pbElement = PbJSONElement.fromBinary(bytes);
Expand Down
13 changes: 9 additions & 4 deletions src/document/crdt/element_rht.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@
* limitations under the License.
*/

import { logger } from '@yorkie-js-sdk/src/util/logger';
import { TimeTicket } from '@yorkie-js-sdk/src/document/time/ticket';
import { CRDTElement } from '@yorkie-js-sdk/src/document/crdt/element';
import { YorkieError, Code } from '@yorkie-js-sdk/src/util/error';

/**
* `ElementRHTNode` is a node of ElementRHT.
Expand Down Expand Up @@ -114,7 +114,10 @@ export class ElementRHT {
*/
public delete(createdAt: TimeTicket, executedAt: TimeTicket): CRDTElement {
if (!this.nodeMapByCreatedAt.has(createdAt.toIDString())) {
logger.fatal(`fail to find ${createdAt.toIDString()}`);
throw new YorkieError(
Code.ErrInvalidArgument,
`fail to find ${createdAt.toIDString()}`,
);
}

const node = this.nodeMapByCreatedAt.get(createdAt.toIDString())!;
Expand Down Expand Up @@ -142,8 +145,10 @@ export class ElementRHT {
element.getCreatedAt().toIDString(),
);
if (!node) {
logger.fatal(`fail to find ${element.getCreatedAt().toIDString()}`);
return;
throw new YorkieError(
Code.ErrInvalidArgument,
`fail to find ${element.getCreatedAt().toIDString()}`,
);
}

const nodeByKey = this.nodeMapByKey.get(node.getStrKey());
Expand Down
20 changes: 15 additions & 5 deletions src/document/crdt/rga_tree_list.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@
* limitations under the License.
*/

import { logger } from '@yorkie-js-sdk/src/util/logger';
import { SplayNode, SplayTree } from '@yorkie-js-sdk/src/util/splay_tree';
import {
InitialTimeTicket,
TimeTicket,
} from '@yorkie-js-sdk/src/document/time/ticket';
import { CRDTElement } from '@yorkie-js-sdk/src/document/crdt/element';
import { Primitive } from '@yorkie-js-sdk/src/document/crdt/primitive';
import { Code, YorkieError } from '@yorkie-js-sdk/src/util/error';

/**
* `RGATreeListNode` is a node of RGATreeList.
Expand Down Expand Up @@ -180,7 +180,10 @@ export class RGATreeList {
): RGATreeListNode {
let node = this.nodeMapByCreatedAt.get(createdAt.toIDString());
if (!node) {
logger.fatal(`cant find the given node: ${createdAt.toIDString()}`);
throw new YorkieError(
Code.ErrInvalidArgument,
`cant find the given node: ${createdAt.toIDString()}`,
);
}

while (
Expand Down Expand Up @@ -232,12 +235,18 @@ export class RGATreeList {
): void {
const prevNode = this.nodeMapByCreatedAt.get(prevCreatedAt.toIDString());
if (!prevNode) {
logger.fatal(`cant find the given node: ${prevCreatedAt.toIDString()}`);
throw new YorkieError(
Code.ErrInvalidArgument,
`cant find the given node: ${prevCreatedAt.toIDString()}`,
);
}

const node = this.nodeMapByCreatedAt.get(createdAt.toIDString());
if (!node) {
logger.fatal(`cant find the given node: ${createdAt.toIDString()}`);
throw new YorkieError(
Code.ErrInvalidArgument,
`cant find the given node: ${createdAt.toIDString()}`,
);
}

if (
Expand Down Expand Up @@ -284,7 +293,8 @@ export class RGATreeList {
element.getCreatedAt().toIDString(),
);
if (!node) {
logger.fatal(
throw new YorkieError(
Code.ErrInvalidArgument,
`fail to find the given createdAt: ${element
.getCreatedAt()
.toIDString()}`,
Expand Down
13 changes: 9 additions & 4 deletions src/document/crdt/rga_tree_split.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
* limitations under the License.
*/

import { logger } from '@yorkie-js-sdk/src/util/logger';
import { ActorID } from '@yorkie-js-sdk/src/document/time/actor_id';
import { Comparator } from '@yorkie-js-sdk/src/util/comparator';
import { SplayNode, SplayTree } from '@yorkie-js-sdk/src/util/splay_tree';
Expand All @@ -26,6 +25,7 @@ import {
TimeTicketStruct,
} from '@yorkie-js-sdk/src/document/time/ticket';
import { GCChild, GCPair, GCParent } from '@yorkie-js-sdk/src/document/crdt/gc';
import { Code, YorkieError } from '@yorkie-js-sdk/src/util/error';

export interface ValueChange<T> {
actor: ActorID;
Expand Down Expand Up @@ -634,7 +634,8 @@ export class RGATreeSplit<T extends RGATreeSplitValue> implements GCParent {
? this.findFloorNodePreferToLeft(absoluteID)
: this.findFloorNode(absoluteID);
if (!node) {
logger.fatal(
throw new YorkieError(
Code.ErrInvalidArgument,
`the node of the given id should be found: ${absoluteID.toTestString()}`,
);
}
Expand Down Expand Up @@ -793,7 +794,8 @@ export class RGATreeSplit<T extends RGATreeSplitValue> implements GCParent {
): RGATreeSplitNode<T> {
let node = this.findFloorNode(id);
if (!node) {
logger.fatal(
throw new YorkieError(
Code.ErrInvalidArgument,
`the node of the given id should be found: ${id.toTestString()}`,
);
}
Expand Down Expand Up @@ -847,7 +849,10 @@ export class RGATreeSplit<T extends RGATreeSplitValue> implements GCParent {
offset: number,
): RGATreeSplitNode<T> | undefined {
if (offset > node.getContentLength()) {
logger.fatal('offset should be less than or equal to length');
throw new YorkieError(
Code.ErrInvalidArgument,
`offset should be less than or equal to length`,
);
}

if (offset === 0) {
Expand Down
7 changes: 5 additions & 2 deletions src/document/crdt/root.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
* limitations under the License.
*/

import { logger } from '@yorkie-js-sdk/src/util/logger';
import {
InitialTimeTicket,
TimeTicket,
Expand All @@ -27,6 +26,7 @@ import { CRDTObject } from '@yorkie-js-sdk/src/document/crdt/object';
import { GCPair } from '@yorkie-js-sdk/src/document/crdt/gc';
import { CRDTText } from '@yorkie-js-sdk/src/document/crdt/text';
import { CRDTTree } from '@yorkie-js-sdk/src/document/crdt/tree';
import { Code, YorkieError } from '@yorkie-js-sdk/src/util/error';

/**
* `CRDTElementPair` is a structure that represents a pair of element and its
Expand Down Expand Up @@ -134,7 +134,10 @@ export class CRDTRoot {
const createdAt = pair.element.getCreatedAt();
const subPath = pair.parent.subPathOf(createdAt);
if (subPath === undefined) {
logger.fatal(`cant find the given element: ${createdAt.toIDString()}`);
throw new YorkieError(
Code.ErrInvalidArgument,
`cant find the given element: ${createdAt.toIDString()}`,
);
}

subPaths.unshift(subPath!);
Expand Down
19 changes: 15 additions & 4 deletions src/document/crdt/tree.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import { Indexable } from '@yorkie-js-sdk/src/document/document';
import type * as Devtools from '@yorkie-js-sdk/src/devtools/types';
import { escapeString } from '@yorkie-js-sdk/src/document/json/strings';
import { GCChild, GCPair, GCParent } from '@yorkie-js-sdk/src/document/crdt/gc';
import { Code, YorkieError } from '@yorkie-js-sdk/src/util/error';

/**
* `TreeNode` represents a node in the tree.
Expand Down Expand Up @@ -223,7 +224,8 @@ export class CRDTTreePos {
const parentNode = tree.findFloorNode(parentID);
let leftNode = tree.findFloorNode(leftSiblingID);
if (!parentNode || !leftNode) {
throw new Error(
throw new YorkieError(
Code.ErrRefused,
`cannot find node of CRDTTreePos(${parentID.toTestString()}, ${leftSiblingID.toTestString()})`,
);
}
Expand Down Expand Up @@ -514,7 +516,10 @@ export class CRDTTreeNode
*/
get value() {
if (!this.isText) {
throw new Error(`cannot get value of element node: ${this.type}`);
throw new YorkieError(
Code.ErrInvalidType,
`cannot get value of element node: ${this.type}`,
);
}

return this._value;
Expand All @@ -525,7 +530,10 @@ export class CRDTTreeNode
*/
set value(v: string) {
if (!this.isText) {
throw new Error(`cannot set value of element node: ${this.type}`);
throw new YorkieError(
Code.ErrInvalidType,
`cannot set value of element node: ${this.type}`,
);
}

this._value = v;
Expand Down Expand Up @@ -1217,7 +1225,10 @@ export class CRDTTree extends CRDTElement implements GCParent {
ticket: TimeTicket,
): void {
// TODO(hackerwins, easylogic): Implement this with keeping references of the nodes.
throw new Error(`not implemented: ${target}, ${source}, ${ticket}`);
throw new YorkieError(
Code.ErrUnimplemented,
`not implemented: ${target}, ${source}, ${ticket}`,
);
}

/**
Expand Down
29 changes: 22 additions & 7 deletions src/document/document.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,7 @@ export class Document<T, P extends Indexable = Indexable> {
} catch (err) {
// drop clone because it is contaminated.
this.clone = undefined;
logger.error(err);

throw err;
} finally {
this.isUpdating = false;
Expand Down Expand Up @@ -847,7 +847,10 @@ export class Document<T, P extends Indexable = Indexable> {
): Unsubscribe {
if (typeof arg1 === 'string') {
if (typeof arg2 !== 'function') {
throw new Error('Second argument must be a callback function');
throw new YorkieError(
Code.ErrInvalidArgument,
'Second argument must be a callback function',
);
}
if (arg1 === 'presence') {
const callback = arg2 as DocEventCallbackMap<P>['presence'];
Expand Down Expand Up @@ -1021,7 +1024,7 @@ export class Document<T, P extends Indexable = Indexable> {
complete,
);
}
throw new Error(`"${arg1}" is not a valid`);
throw new YorkieError(Code.ErrInvalidArgument, `"${arg1}" is not a valid`);
}

/**
Expand Down Expand Up @@ -1761,11 +1764,17 @@ export class Document<T, P extends Indexable = Indexable> {
*/
private undo(): void {
if (this.isUpdating) {
throw new Error('Undo is not allowed during an update');
throw new YorkieError(
Code.ErrRefused,
'Undo is not allowed during an update',
);
}
const undoOps = this.internalHistory.popUndo();
if (undoOps === undefined) {
throw new Error('There is no operation to be undone');
throw new YorkieError(
Code.ErrRefused,
'There is no operation to be undone',
);
}

this.ensureClone();
Expand Down Expand Up @@ -1855,12 +1864,18 @@ export class Document<T, P extends Indexable = Indexable> {
*/
private redo(): void {
if (this.isUpdating) {
throw new Error('Redo is not allowed during an update');
throw new YorkieError(
Code.ErrRefused,
'Redo is not allowed during an update',
);
}

const redoOps = this.internalHistory.popRedo();
if (redoOps === undefined) {
throw new Error('There is no operation to be redone');
throw new YorkieError(
Code.ErrRefused,
'There is no operation to be redone',
);
}

this.ensureClone();
Expand Down
14 changes: 9 additions & 5 deletions src/document/json/counter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
* limitations under the License.
*/

import { logger } from '@yorkie-js-sdk/src/util/logger';
import { TimeTicket } from '@yorkie-js-sdk/src/document/time/ticket';
import { ChangeContext } from '@yorkie-js-sdk/src/document/change/context';
import { Primitive } from '@yorkie-js-sdk/src/document/crdt/primitive';
Expand All @@ -25,6 +24,7 @@ import {
CRDTCounter,
} from '@yorkie-js-sdk/src/document/crdt/counter';
import type * as Devtools from '@yorkie-js-sdk/src/devtools/types';
import { Code, YorkieError } from '@yorkie-js-sdk/src/util/error';

/**
* `Counter` is a custom data type that is used to counter.
Expand Down Expand Up @@ -78,9 +78,10 @@ export class Counter {
*/
public increase(v: number | Long): Counter {
if (!this.context || !this.counter) {
logger.fatal('it is not initialized yet');
// @ts-ignore
return;
throw new YorkieError(
Code.ErrNotInitialized,
'Counter is not initialized yet',
);
}

const ticket = this.context.issueTimeTicket();
Expand All @@ -105,7 +106,10 @@ export class Counter {
*/
public toJSForTest(): Devtools.JSONElement {
if (!this.context || !this.counter) {
throw new Error('it is not initialized yet');
throw new YorkieError(
Code.ErrNotInitialized,
'Counter is not initialized yet',
);
}

return this.counter.toJSForTest();
Expand Down
Loading

0 comments on commit 267878f

Please sign in to comment.