stephen/apartment-dashboard-api
1
0
Fork 0
Code Issues 1 Pull Requests Actions Releases Wiki Activity

Replace console.error with structured logger in recordScraperRun
All checks were successful
CI/CD Pipeline - Apartment API / Send Webhook Notification (pull_request) Successful in 3s
Details
CI/CD Pipeline - Apartment API / Build & Push Image (pull_request) Has been skipped
Details
CI/CD Pipeline - Apartment API / Scan Dependencies (pull_request) Successful in 13s
Details
CI/CD Pipeline - Apartment API / Run Linting (pull_request) Successful in 9m36s
Details
CI/CD Pipeline - Apartment API / Run Tests (pull_request) Successful in 9m51s
Details
CI/CD Pipeline - Apartment API / Deploy to Production (pull_request) Has been skipped
Details

Browse Source 
Switch error logging from console.error to the injected logger.error
pattern for consistency with the rest of the scraper service layer.
Update tests to verify logger.error is called instead of console.error.
This commit is contained in:
Stephen Minakian
2026-02-06 01:41:50 -07:00
parent 74dc62c53f
commit 48512e01ba
2 changed files with 27 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

54
__tests__/scraper/recordScraperRun.test.js
View File

@ -19,6 +19,7 @@ let recordScraperRun;
let mongoServer; let mongoServer;
let client; let client;
let db; let db;
let logger;
beforeAll(async () => { beforeAll(async () => {
mongoServer = await MongoMemoryServer.create(); mongoServer = await MongoMemoryServer.create();
@ -42,6 +43,9 @@ beforeEach(async () => {
for (const col of collections) { for (const col of collections) {
await db.collection(col.name).deleteMany({}); await db.collection(col.name).deleteMany({});
} }
// Create fresh mock logger for each test
logger = { info: jest.fn(), warn: jest.fn(), error: jest.fn() };
}); });
describe('recordScraperRun', () => { describe('recordScraperRun', () => {
@ -74,7 +78,7 @@ describe('recordScraperRun', () => {
it('should insert a document into the scraper_runs collection', async () => { it('should insert a document into the scraper_runs collection', async () => {
const runData = createSampleRunData(); const runData = createSampleRunData();
await recordScraperRun(db, runData); await recordScraperRun(db, runData, logger);
const docs = await db.collection(SCRAPER_RUNS_COLLECTION).find({}).toArray(); const docs = await db.collection(SCRAPER_RUNS_COLLECTION).find({}).toArray();
expect(docs).toHaveLength(1); expect(docs).toHaveLength(1);
@ -83,7 +87,7 @@ describe('recordScraperRun', () => {
it('should include a recordedAt field in the inserted document', async () => { it('should include a recordedAt field in the inserted document', async () => {
const runData = createSampleRunData(); const runData = createSampleRunData();
await recordScraperRun(db, runData); await recordScraperRun(db, runData, logger);
const doc = await db.collection(SCRAPER_RUNS_COLLECTION).findOne({}); const doc = await db.collection(SCRAPER_RUNS_COLLECTION).findOne({});
expect(doc.recordedAt).toBeDefined(); expect(doc.recordedAt).toBeDefined();
@ -102,7 +106,7 @@ describe('recordScraperRun', () => {
duration: 9999 duration: 9999
}); });
await recordScraperRun(db, runData); await recordScraperRun(db, runData, logger);
const doc = await db.collection(SCRAPER_RUNS_COLLECTION).findOne({}); const doc = await db.collection(SCRAPER_RUNS_COLLECTION).findOne({});
expect(doc.jobId).toBe('preserve-test-001'); expect(doc.jobId).toBe('preserve-test-001');
@ -120,7 +124,7 @@ describe('recordScraperRun', () => {
staleUnitsCount: 3 staleUnitsCount: 3
}); });
await recordScraperRun(db, runData); await recordScraperRun(db, runData, logger);
const doc = await db.collection(SCRAPER_RUNS_COLLECTION).findOne({}); const doc = await db.collection(SCRAPER_RUNS_COLLECTION).findOne({});
expect(doc.unitsProcessed).toBe(42); expect(doc.unitsProcessed).toBe(42);
@ -136,7 +140,7 @@ describe('recordScraperRun', () => {
completedAt: '2026-02-06T10:00:15.500Z' completedAt: '2026-02-06T10:00:15.500Z'
}); });
await recordScraperRun(db, runData); await recordScraperRun(db, runData, logger);
const doc = await db.collection(SCRAPER_RUNS_COLLECTION).findOne({}); const doc = await db.collection(SCRAPER_RUNS_COLLECTION).findOne({});
expect(doc.startedAt).toBe('2026-02-06T10:00:00.000Z'); expect(doc.startedAt).toBe('2026-02-06T10:00:00.000Z');
@ -146,7 +150,7 @@ describe('recordScraperRun', () => {
it('should preserve dryRun flag', async () => { it('should preserve dryRun flag', async () => {
const runData = createSampleRunData({ dryRun: true }); const runData = createSampleRunData({ dryRun: true });
await recordScraperRun(db, runData); await recordScraperRun(db, runData, logger);
const doc = await db.collection(SCRAPER_RUNS_COLLECTION).findOne({}); const doc = await db.collection(SCRAPER_RUNS_COLLECTION).findOne({});
expect(doc.dryRun).toBe(true); expect(doc.dryRun).toBe(true);
@ -158,7 +162,7 @@ describe('recordScraperRun', () => {
errors: ['Connection timeout', 'Retry exhausted'] errors: ['Connection timeout', 'Retry exhausted']
}); });
await recordScraperRun(db, runData); await recordScraperRun(db, runData, logger);
const doc = await db.collection(SCRAPER_RUNS_COLLECTION).findOne({}); const doc = await db.collection(SCRAPER_RUNS_COLLECTION).findOne({});
expect(doc.errors).toEqual(['Connection timeout', 'Retry exhausted']); expect(doc.errors).toEqual(['Connection timeout', 'Retry exhausted']);
@ -167,7 +171,7 @@ describe('recordScraperRun', () => {
it('should preserve empty errors array for successful runs', async () => { it('should preserve empty errors array for successful runs', async () => {
const runData = createSampleRunData({ errors: [] }); const runData = createSampleRunData({ errors: [] });
await recordScraperRun(db, runData); await recordScraperRun(db, runData, logger);
const doc = await db.collection(SCRAPER_RUNS_COLLECTION).findOne({}); const doc = await db.collection(SCRAPER_RUNS_COLLECTION).findOne({});
expect(doc.errors).toEqual([]); expect(doc.errors).toEqual([]);
@ -190,7 +194,7 @@ describe('recordScraperRun', () => {
}; };
// This should NOT throw // This should NOT throw
await expect(recordScraperRun(mockDb, runData)).resolves.not.toThrow(); await expect(recordScraperRun(mockDb, runData, logger)).resolves.not.toThrow();
}); });
it('should log error message when insert fails', async () => { it('should log error message when insert fails', async () => {
@ -203,17 +207,12 @@ describe('recordScraperRun', () => {
collection: jest.fn().mockReturnValue(mockCollection) collection: jest.fn().mockReturnValue(mockCollection)
}; };
// Spy on console.error await recordScraperRun(mockDb, runData, logger);
const consoleSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
await recordScraperRun(mockDb, runData); expect(logger.error).toHaveBeenCalledWith(
'Failed to record scraper run',
expect(consoleSpy).toHaveBeenCalledWith( { errorMessage: 'Disk full' }
expect.stringContaining('Failed to record scraper run'),
expect.stringContaining('Disk full')
); );
consoleSpy.mockRestore();
}); });
}); });
@ -231,13 +230,8 @@ describe('recordScraperRun', () => {
collection: jest.fn().mockReturnValue(mockCollection) collection: jest.fn().mockReturnValue(mockCollection)
}; };
// Suppress console.error for clean test output const result = await recordScraperRun(mockDb, runData, logger);
const consoleSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
const result = await recordScraperRun(mockDb, runData);
expect(result).toBeNull(); expect(result).toBeNull();
consoleSpy.mockRestore();
}); });
}); });
@ -248,7 +242,7 @@ describe('recordScraperRun', () => {
it('should return the insertOne result object', async () => { it('should return the insertOne result object', async () => {
const runData = createSampleRunData(); const runData = createSampleRunData();
const result = await recordScraperRun(db, runData); const result = await recordScraperRun(db, runData, logger);
expect(result).toBeDefined(); expect(result).toBeDefined();
expect(result).not.toBeNull(); expect(result).not.toBeNull();
@ -257,7 +251,7 @@ describe('recordScraperRun', () => {
it('should return result with acknowledged property', async () => { it('should return result with acknowledged property', async () => {
const runData = createSampleRunData(); const runData = createSampleRunData();
const result = await recordScraperRun(db, runData); const result = await recordScraperRun(db, runData, logger);
expect(result.acknowledged).toBe(true); expect(result.acknowledged).toBe(true);
}); });
@ -265,7 +259,7 @@ describe('recordScraperRun', () => {
it('should return result with insertedId', async () => { it('should return result with insertedId', async () => {
const runData = createSampleRunData(); const runData = createSampleRunData();
const result = await recordScraperRun(db, runData); const result = await recordScraperRun(db, runData, logger);
expect(result.insertedId).toBeDefined(); expect(result.insertedId).toBeDefined();
}); });
@ -278,7 +272,7 @@ describe('recordScraperRun', () => {
it('should set recordedAt as a Date instance', async () => { it('should set recordedAt as a Date instance', async () => {
const runData = createSampleRunData(); const runData = createSampleRunData();
await recordScraperRun(db, runData); await recordScraperRun(db, runData, logger);
const doc = await db.collection(SCRAPER_RUNS_COLLECTION).findOne({}); const doc = await db.collection(SCRAPER_RUNS_COLLECTION).findOne({});
expect(doc.recordedAt).toBeInstanceOf(Date); expect(doc.recordedAt).toBeInstanceOf(Date);
@ -288,7 +282,7 @@ describe('recordScraperRun', () => {
const beforeTime = new Date(); const beforeTime = new Date();
const runData = createSampleRunData(); const runData = createSampleRunData();
await recordScraperRun(db, runData); await recordScraperRun(db, runData, logger);
const afterTime = new Date(); const afterTime = new Date();
@ -300,7 +294,7 @@ describe('recordScraperRun', () => {
it('should not overwrite any existing runData fields with recordedAt', async () => { it('should not overwrite any existing runData fields with recordedAt', async () => {
const runData = createSampleRunData(); const runData = createSampleRunData();
await recordScraperRun(db, runData); await recordScraperRun(db, runData, logger);
const doc = await db.collection(SCRAPER_RUNS_COLLECTION).findOne({}); const doc = await db.collection(SCRAPER_RUNS_COLLECTION).findOne({});

5
services/scraperService.js
View File

@ -626,9 +626,10 @@ async function updateDailySummary(db, summaryData, logger) {
* *
* @param {Db} db - MongoDB database instance * @param {Db} db - MongoDB database instance
* @param {Object} runData - Run data to record (jobId, trigger, status, duration, etc.) * @param {Object} runData - Run data to record (jobId, trigger, status, duration, etc.)
* @param {Object} logger - Logger instance
* @returns {Promise<Object|null>} Insert result, or null on failure * @returns {Promise<Object|null>} Insert result, or null on failure
*/ */
async function recordScraperRun(db, runData) { async function recordScraperRun(db, runData, logger) {
const collection = db.collection(config.COLLECTIONS.SCRAPER_RUNS); const collection = db.collection(config.COLLECTIONS.SCRAPER_RUNS);
try { try {
@ -641,7 +642,7 @@ async function recordScraperRun(db, runData) {
} catch (error) { } catch (error) {
// Log but don't throw - recording history should not break scraper // Log but don't throw - recording history should not break scraper
console.error('Failed to record scraper run:', error.message); logger.error('Failed to record scraper run', { errorMessage: error.message });
return null; return null;
} }
} }