Skip to content

Commit b0e9ba2

Browse files
ekremneyclaude
andcommitted
fix(data-access): surface PG error details in logs and stop leaking credentials
#logAndThrowError was passing `this` (the entire BaseCollection instance including JWT headers) as DataAccessError details, leaking credentials to logs. PG error fields (code, message, details, hint) were never surfaced in the log string — only visible in Coralogix deep serialization. - Rewrite #logAndThrowError to read code/message/details/hint directly from the PostgrestError cause and log a human-readable string - Pass { entityName, tableName } instead of `this` as DataAccessError details - Fix updateByKeys and batchGetByKeys to use #logAndThrowError for consistent error logging instead of direct throw Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 355f09c commit b0e9ba2

File tree

2 files changed

+28
-15
lines changed

2 files changed

+28
-15
lines changed

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

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -104,21 +104,29 @@ class BaseCollection {
104104
#isInvalidInputError(error) {
105105
let current = error;
106106
while (current) {
107-
if (current?.code === '22P02') {
108-
return true;
109-
}
107+
if (current?.code === '22P02') return true;
110108
current = current.cause;
111109
}
112110
return false;
113111
}
114112

115113
#logAndThrowError(message, cause) {
116-
const error = new DataAccessError(message, this, cause);
117-
this.log.error(`Base Collection Error [${this.entityName}]`, error);
118-
if (isNonEmptyArray(error.cause?.fields)) {
119-
this.log.error(`Validation errors: ${JSON.stringify(error.cause.fields)}`);
114+
const parts = [message];
115+
if (cause?.code) parts.push(`[${cause.code}] ${cause.message}`);
116+
if (cause?.details) parts.push(cause.details);
117+
if (cause?.hint) parts.push(`hint: ${cause.hint}`);
118+
119+
this.log.error(`[${this.entityName}] ${parts.join(' - ')}`);
120+
121+
if (isNonEmptyArray(cause?.fields)) {
122+
this.log.error(`Validation errors: ${JSON.stringify(cause.fields)}`);
120123
}
121-
throw error;
124+
125+
throw new DataAccessError(
126+
message,
127+
{ entityName: this.entityName, tableName: this.tableName },
128+
cause,
129+
);
122130
}
123131

124132
#initializeCollectionMethods() {
@@ -601,6 +609,7 @@ class BaseCollection {
601609
return isNonEmptyObject(item);
602610
}
603611

612+
// eslint-disable-next-line consistent-return
604613
async batchGetByKeys(keys, options = {}) {
605614
guardArray('keys', keys, this.entityName, 'any');
606615

@@ -677,8 +686,11 @@ class BaseCollection {
677686
unprocessed: [],
678687
};
679688
} catch (error) {
680-
this.log.error(`Failed to batch get by keys [${this.entityName}]`, error);
681-
throw new DataAccessError('Failed to batch get by keys', this, error);
689+
/* c8 ignore next 3 -- re-throw guard for already-wrapped errors */
690+
if (error instanceof DataAccessError) {
691+
throw error;
692+
}
693+
this.#logAndThrowError('Failed to batch get by keys', error);
682694
}
683695
}
684696

@@ -856,7 +868,7 @@ class BaseCollection {
856868

857869
const { error } = await query.select().maybeSingle();
858870
if (error) {
859-
throw new DataAccessError('Failed to update entity', this, error);
871+
this.#logAndThrowError('Failed to update entity', error);
860872
}
861873
}
862874

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1191,7 +1191,9 @@ describe('BaseCollection', () => {
11911191
});
11921192

11931193
await expect(baseCollectionInstance.batchGetByKeys(keys)).to.be.rejectedWith(DataAccessError);
1194-
expect(mockLogger.error).to.have.been.calledWith('Failed to batch get by keys [mockEntityModel]', error);
1194+
expect(mockLogger.error).to.have.been.calledWith(
1195+
sinon.match('[mockEntityModel] Failed to batch get by keys'),
1196+
);
11951197
});
11961198

11971199
it('should handle null records in results', async () => {
@@ -1410,10 +1412,9 @@ describe('BaseCollection', () => {
14101412
await expect(baseCollectionInstance.removeByIndexKeys(keys))
14111413
.to.be.rejectedWith(DataAccessError, 'Failed to remove by index keys');
14121414

1413-
// The error logging uses the format "Base Collection Error [entityName]"
1415+
// The error logging surfaces the error message with entity context
14141416
expect(mockLogger.error).to.have.been.calledWith(
1415-
'Base Collection Error [mockEntityModel]',
1416-
sinon.match.instanceOf(DataAccessError),
1417+
sinon.match('[mockEntityModel] Failed to remove by index keys'),
14171418
);
14181419
});
14191420

0 commit comments

Comments
 (0)