chore: pr review

packages/spacecat-shared-data-access/src/v2/models/base/base.collection.js

@@ -296,6 +296,7 @@ class BaseCollection {
    * @param {QueryOptions} [options] - Additional options for the query.
    * Additional options for the query.
    * @return {Promise<BaseModel|null>}
+   * @throws {DataAccessError} - Throws an error if the sort keys are not provided.
    */
   async findByAll(sortKeys = {}, options = {}) {
     if (!isObject(sortKeys)) {
@@ -315,7 +316,7 @@ class BaseCollection {
    * @param {string} id - The unique identifier of the entity to be found.
    * @returns {Promise<BaseModel|null>} - A promise that resolves to an instance of
    * the model if found, otherwise null.
-   * @throws {Error} - Throws an error if the ID is not provided.
+   * @throws {ValidationError} - Throws an error if the ID is not provided.
    */
   async findById(id) {
     guardId(this.idName, id, this.entityName);
@@ -330,6 +331,7 @@ class BaseCollection {
    * @param {Object} keys - The index keys to use for the query.
    * @param {{index?: string, attributes?: string[]}} [options] - Additional options for the query.
    * @returns {Promise<BaseModel|null>} - A promise that resolves to the model instance or null.
+   * @throws {DataAccessError} - Throws an error if retrieving the entity fails.
    * @async
    */
   async findByIndexKeys(keys, options = {}) {
@@ -343,7 +345,8 @@ class BaseCollection {
    * @async
    * @param {Object} item - The data for the entity to be created.
    * @returns {Promise<BaseModel>} - A promise that resolves to the created model instance.
-   * @throws {Error} - Throws an error if the data is invalid or if the creation process fails.
+   * @throws {DataAccessError} - Throws an error if the data is invalid or if the
+   * creation process fails.
    */
   async create(item) {
     if (!isNonEmptyObject(item)) {

packages/spacecat-shared-data-access/src/v2/models/base/base.model.js

@@ -223,7 +223,7 @@ class BaseModel {
    * @async
    * @returns {Promise<BaseModel>} - A promise that resolves to the current instance of the entity
    * after it and its dependents have been removed.
-   * @throws {Error} - Throws an error if the schema does not allow removal
+   * @throws {DataAccessError} - Throws an error if the schema does not allow removal
    * or if the removal operation fails.
    */
   async remove() {
@@ -239,7 +239,7 @@ class BaseModel {
    * This method does not check if the schema allows removal in order to be able to remove
    * dependents even if the schema does not allow removal.
    * @return {Promise<BaseModel>}
-   * @throws {Error} - Throws an error if the removal operation fails.
+   * @throws {DataAccessError} - Throws an error if the removal operation fails.
    * @protected
    */
   async _remove() {
@@ -258,7 +258,11 @@ class BaseModel {
       return this;
     } catch (error) {
       this.log.error('Failed to remove record', error);
-      throw error;
+      throw new DataAccessError(
+        `Failed to remove entity ${this.entityName} with ID ${this.getId()}`,
+        this,
+        error,
+      );
     }
   }
 
@@ -268,7 +272,7 @@ class BaseModel {
    * @async
    * @returns {Promise<BaseModel>} - A promise that resolves to the current instance of the entity
    * after it has been saved.
-   * @throws {Error} - Throws an error if the save operation fails.
+   * @throws {DataAccessError} - Throws an error if the save operation fails.
    */
   async save() {
     // todo: validate associations
@@ -281,7 +285,11 @@ class BaseModel {
       return this;
     } catch (error) {
       this.log.error('Failed to save record', error);
-      throw error;
+      throw new DataAccessError(
+        `Failed to to save entity ${this.entityName} with ID ${this.getId()}`,
+        this,
+        error,
+      );
     }
   }
 

packages/spacecat-shared-data-access/src/v2/models/base/entity.registry.js

@@ -93,7 +93,8 @@ class EntityRegistry {
    * Gets a collection instance by its name.
    * @param {string} collectionName - The name of the collection to retrieve.
    * @returns {Object} - The requested collection instance.
-   * @throws {Error} - Throws an error if the collection with the specified name is not found.
+   * @throws {DataAccessError} - Throws an error if the collection with the
+   * specified name is not found.
    */
   getCollection(collectionName) {
     const collection = this.collections.get(collectionName);

packages/spacecat-shared-data-access/src/v2/models/base/schema.builder.js

@@ -88,9 +88,9 @@ class SchemaBuilder {
    * @param {BaseModel} modelClass - The model class for this entity.
    * @param {BaseCollection} collectionClass - The collection class for this entity.
    * @param {number} schemaVersion - A positive integer representing the schema's version.
-   * @throws {Error} If entityName is not a non-empty string.
-   * @throws {Error} If schemaVersion is not a positive integer.
-   * @throws {Error} If serviceName is not a non-empty string.
+   * @throws {SchemaBuilderError} If entityName is not a non-empty string.
+   * @throws {SchemaBuilderError} If schemaVersion is not a positive integer.
+   * @throws {SchemaBuilderError} If serviceName is not a non-empty string.
    */
   constructor(modelClass, collectionClass, schemaVersion = 1) {
     if (!modelClass || !(modelClass.prototype instanceof BaseModel)) {
@@ -174,7 +174,7 @@ class SchemaBuilder {
    * This should only be used in special cases.
    *
    * @param {Array<string>} sortKeys - The attributes to form the sort key.
-   * @throws {Error} If sortKeys are not provided or are not a non-empty array.
+   * @throws {SchemaBuilderError} If sortKeys are not provided or are not a non-empty array.
    * @return {SchemaBuilder}
    */
   withPrimarySortKeys(sortKeys) {
@@ -194,7 +194,7 @@ class SchemaBuilder {
    * remove is called implicitly when the entity is removed
    * as part of parent entity remove (dependents).
    * @param {boolean} allow - Whether to allow removes.
-   * @throws {Error} If allow is not a boolean.
+   * @throws {SchemaBuilderError} If allow is not a boolean.
    * @return {SchemaBuilder}
    */
   allowRemove(allow) {
@@ -212,7 +212,7 @@ class SchemaBuilder {
    * not prevent updates at the database level, but rather
    * at the application level.
    * @param {boolean} allow - Whether to allow updates.
-   * @throws {Error} If allow is not a boolean.
+   * @throws {SchemaBuilderError} If allow is not a boolean.
    * @return {SchemaBuilder}
    */
   allowUpdates(allow) {
@@ -230,7 +230,7 @@ class SchemaBuilder {
    * @param {string} name - The attribute name.
    * @param {object} data - The attribute definition (type, required, validation, etc.).
    * @returns {SchemaBuilder} Returns this builder for method chaining.
-   * @throws {Error} If name is not non-empty or data is not an object.
+   * @throws {SchemaBuilderError} If name is not non-empty or data is not an object.
    */
   addAttribute(name, data) {
     if (!hasText(name)) {
@@ -253,7 +253,7 @@ class SchemaBuilder {
    *
    * @param {Array<string>} sortKeys - The attributes to form the sort key.
    * @returns {SchemaBuilder} Returns this builder for method chaining.
-   * @throws {Error} If composite attribute names or template are not provided.
+   * @throws {SchemaBuilderError} If composite attribute names or template are not provided.
    */
   addAllIndex(sortKeys) {
     if (!isNonEmptyArray(sortKeys)) {
@@ -276,7 +276,7 @@ class SchemaBuilder {
    * (e.g., { composite: [attributeName] }).
    * @param {object} sortKey - The sort key definition.
    * @returns {SchemaBuilder} Returns this builder for method chaining.
-   * @throws {Error} If index name is reserved or pk/sk configs are invalid.
+   * @throws {SchemaBuilderError} If index name is reserved or pk/sk configs are invalid.
    */
   addIndex(partitionKey, sortKey) {
     if (!isNonEmptyObject(partitionKey)) {
@@ -304,7 +304,7 @@ class SchemaBuilder {
    * @param {boolean} [options.removeDependents=false] - Whether to remove dependent entities
    * on delete. Only applies to HAS_MANY and HAS_ONE references.
    * @returns {SchemaBuilder} Returns this builder for method chaining.
-   * @throws {Error} If type or entityName are invalid.
+   * @throws {SchemaBuilderError} If type or entityName are invalid.
    */
   addReference(type, entityName, sortKeys = [], options = {}) {
     if (!Reference.isValidType(type)) {

packages/spacecat-shared-data-access/src/v2/models/base/schema.js

@@ -188,7 +188,7 @@ class Schema {
    * index is found, we fall back to the "all" index, then the "primary".
    *
    * @param {Object} keys - The keys to search for.
-   * @return {*|string} - The index name.
+   * @return {string} - The index name.
    */
   findIndexNameByKeys(keys) {
     const { ALL, PRIMARY } = this.getIndexTypes();
@@ -313,6 +313,7 @@ class Schema {
    *
    * @param {BaseModel|BaseCollection} entity - The entity for which to generate accessors.
    * @param {Object} [log] - The logger to use for logging information
+   * @throws {SchemaError} - Throws an error if the entity is not a BaseModel or BaseCollection.
    * @return {Object[]}
    */
   toAccessorConfigs(entity, log = console) {

packages/spacecat-shared-data-access/test/unit/v2/models/base/base.model.test.js

@@ -250,7 +250,7 @@ describe('BaseModel', () => { /* eslint-disable no-underscore-dangle */
       const error = new Error('Remove failed');
       mockElectroService.entities.opportunity.remove.returns({ go: () => Promise.reject(error) });
 
-      await expect(baseModelInstance.remove()).to.be.rejectedWith('Remove failed');
+      await expect(baseModelInstance.remove()).to.be.rejectedWith('Failed to remove entity opportunity with ID 12345');
       expect(mockLogger.error.calledOnce).to.be.true;
     });
 
@@ -274,7 +274,7 @@ describe('BaseModel', () => { /* eslint-disable no-underscore-dangle */
       const error = new Error('Save failed');
       baseModelInstance.patcher.save = stub().returns(Promise.reject(error));
 
-      await expect(baseModelInstance.save()).to.be.rejectedWith('Save failed');
+      await expect(baseModelInstance.save()).to.be.rejectedWith('Failed to to save entity opportunity with ID 12345');
       expect(mockLogger.error.calledOnce).to.be.true;
     });
   });