mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 00:11:37 -06:00
fix(scanner): surface Book Dock finalize error details
Log sanitized failure codes and stages while keeping database queries and filesystem paths out of requester-visible messages. Ref #1367
This commit is contained in:
@@ -10,6 +10,7 @@ vi.mock('fs/promises', () => ({
|
||||
|
||||
import { BadRequestException, NotFoundException } from '@nestjs/common';
|
||||
import { access, lstat, readdir, readFile, stat, unlink } from 'fs/promises';
|
||||
import { DatabaseError } from 'pg';
|
||||
|
||||
import {
|
||||
DEFAULT_UPLOAD_PATTERN_BOOK_PER_FILE,
|
||||
@@ -1953,10 +1954,12 @@ describe('BookDockFinalizeService', () => {
|
||||
it('does not put a database error into the message a requester reads', async () => {
|
||||
const harness = makeService();
|
||||
const row = makeRow({ targetLibraryId: 5, targetFolderId: 9, unitDirectory: '/dock/request-7-Dune' });
|
||||
const databaseError = Object.assign(new DatabaseError('syntax error at or near "asc"', 0, 'error'), { code: '42601' });
|
||||
const failure = Object.assign(new Error('Failed query: select "id" from "book_dock_unit_files" where ...'), {
|
||||
cause: Object.assign(new Error('syntax error at or near "asc"'), { code: '42601' }),
|
||||
cause: databaseError,
|
||||
});
|
||||
harness.repo.findUnitFiles.mockRejectedValue(failure);
|
||||
const warn = vi.spyOn((harness.service as any).logger, 'warn').mockImplementation(() => {});
|
||||
vi.spyOn(harness.service as never, 'findLibraryOrFail').mockResolvedValue({
|
||||
id: 5,
|
||||
name: 'Books',
|
||||
@@ -1973,6 +1976,51 @@ describe('BookDockFinalizeService', () => {
|
||||
expect(result.message).toBe('Filing this book failed inside BookOrbit. Check the server log for the cause.');
|
||||
expect(result.message).not.toContain('select');
|
||||
expect(result.message).not.toContain('book_dock_unit_files');
|
||||
expect(warn).toHaveBeenCalledWith(expect.stringContaining('errorClass=DatabaseError errorCode=42601'));
|
||||
expect(warn).toHaveBeenCalledWith(expect.stringContaining('error="syntax error at or near \\"asc\\""'));
|
||||
expect(warn).not.toHaveBeenCalledWith(expect.stringContaining('book_dock_unit_files'));
|
||||
});
|
||||
|
||||
it.each(['EXDEV', 'EPERM', 'EACCES', 'ENOENT'])('logs %s from a failed file move without exposing paths to the requester', async (code) => {
|
||||
const harness = makeService();
|
||||
const row = makeRow({ absolutePath: '/dock/book.epub' });
|
||||
const prepared = {
|
||||
fileId: row.id,
|
||||
fileName: row.fileName,
|
||||
row,
|
||||
status: 'ready',
|
||||
destPath: '/library/book.epub',
|
||||
library: { id: 5 },
|
||||
folder: { id: 9, path: '/library' },
|
||||
format: 'epub',
|
||||
placement: [
|
||||
{
|
||||
sourcePath: '/dock/book.epub',
|
||||
destPath: '/library/book.epub',
|
||||
format: 'epub',
|
||||
role: 'content',
|
||||
sortOrder: 0,
|
||||
},
|
||||
],
|
||||
};
|
||||
const failure = Object.assign(new Error(`${code}: move '/dock/book.epub' -> '/library/book.epub'`), { code });
|
||||
harness.storage.moveToPath.mockRejectedValueOnce(failure);
|
||||
vi.spyOn(harness.service as never, 'classifyDestination').mockResolvedValue(prepared as never);
|
||||
const warn = vi.spyOn((harness.service as any).logger, 'warn').mockImplementation(() => {});
|
||||
|
||||
const result = await (harness.service as any).finalizePreparedCandidate(prepared, new Map());
|
||||
|
||||
expect(result).toEqual({
|
||||
fileId: 1,
|
||||
fileName: 'book.epub',
|
||||
success: false,
|
||||
message: 'Filing this book failed inside BookOrbit. Check the server log for the cause.',
|
||||
});
|
||||
expect(result.message).not.toContain('/dock');
|
||||
expect(result.message).not.toContain('/library');
|
||||
expect(warn).toHaveBeenCalledExactlyOnceWith(
|
||||
`[book_dock.finalize] [fail] fileId=1 stage=filing errorClass=Error errorCode=${code} error="${code}" - Book Dock file finalization failed`,
|
||||
);
|
||||
});
|
||||
|
||||
it('cleanupBookDockRecord deletes cover files and bucket row id', async () => {
|
||||
@@ -2047,6 +2095,7 @@ describe('BookDockFinalizeService', () => {
|
||||
|
||||
it('reports non-ENOENT destination access failures without moving the file', async () => {
|
||||
const { service, storage } = makeService();
|
||||
const warn = vi.spyOn((service as any).logger, 'warn').mockImplementation(() => {});
|
||||
const analysis = {
|
||||
fileId: 1,
|
||||
fileName: 'book.epub',
|
||||
@@ -2061,7 +2110,13 @@ describe('BookDockFinalizeService', () => {
|
||||
|
||||
const classified = await (service as any).classifyDestination(analysis, new Map());
|
||||
|
||||
expect(classified).toMatchObject({ status: 'error', message: 'permission denied' });
|
||||
expect(classified).toMatchObject({
|
||||
status: 'error',
|
||||
message: 'Filing this book failed inside BookOrbit. Check the server log for the cause.',
|
||||
});
|
||||
expect(warn).toHaveBeenCalledExactlyOnceWith(
|
||||
'[book_dock.finalize] [fail] fileId=1 stage=destination_check errorClass=Error errorCode=EACCES error="EACCES" - Book Dock file finalization failed',
|
||||
);
|
||||
expect(storage.moveToPath).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
|
||||
@@ -14,6 +14,7 @@ import { basename, dirname, extname, isAbsolute, join, relative, resolve, sep }
|
||||
import { access as fsAccess, readFile, rmdir, stat, unlink } from 'fs/promises';
|
||||
import { eq, inArray } from 'drizzle-orm';
|
||||
import { NodePgDatabase } from 'drizzle-orm/node-postgres';
|
||||
import { DatabaseError } from 'pg';
|
||||
|
||||
import type {
|
||||
AudiobookChapter,
|
||||
@@ -424,7 +425,7 @@ export class BookDockFinalizeService implements OnModuleInit, OnApplicationBoots
|
||||
return { fileId: row.id, fileName: row.fileName, newName, success: true, bookId };
|
||||
} catch (err) {
|
||||
const message = resolveFinalizeErrorMessage(err);
|
||||
this.logger.warn(`Finalize failed for Book Dock file ${row.id}: ${message}`);
|
||||
this.logFinalizeFailure(row.id, 'filing', err);
|
||||
return { fileId: row.id, fileName: row.fileName, success: false, message };
|
||||
}
|
||||
}
|
||||
@@ -605,11 +606,13 @@ export class BookDockFinalizeService implements OnModuleInit, OnApplicationBoots
|
||||
|
||||
return { fileId: row.id, fileName: row.fileName, row, status: 'ready', newName, library, folder, format, destPath, placement, bookFolderPath };
|
||||
} catch (error) {
|
||||
const status = classifyFinalizePreviewError(error);
|
||||
if (status === 'error') this.logFinalizeFailure(row.id, 'target_resolution', error);
|
||||
return {
|
||||
fileId: row.id,
|
||||
fileName: row.fileName,
|
||||
row,
|
||||
status: classifyFinalizePreviewError(error),
|
||||
status,
|
||||
message: resolveFinalizeErrorMessage(error),
|
||||
};
|
||||
}
|
||||
@@ -795,6 +798,7 @@ export class BookDockFinalizeService implements OnModuleInit, OnApplicationBoots
|
||||
await fsAccess(analysis.destPath);
|
||||
} catch (error) {
|
||||
if ((error as NodeJS.ErrnoException).code === 'ENOENT') return analysis;
|
||||
this.logFinalizeFailure(analysis.fileId, 'destination_check', error);
|
||||
return {
|
||||
...analysis,
|
||||
status: 'error',
|
||||
@@ -819,6 +823,13 @@ export class BookDockFinalizeService implements OnModuleInit, OnApplicationBoots
|
||||
};
|
||||
}
|
||||
|
||||
private logFinalizeFailure(fileId: number, stage: string, error: unknown): void {
|
||||
const details = finalizeErrorLogDetails(error);
|
||||
this.logger.warn(
|
||||
`[book_dock.finalize] [fail] fileId=${fileId} stage=${stage} errorClass=${details.errorClass} errorCode=${details.errorCode} error="${sanitizeLogValue(details.message)}" - Book Dock file finalization failed`,
|
||||
);
|
||||
}
|
||||
|
||||
private destinationKey(libraryId: number, absolutePath: string): string {
|
||||
return `${libraryId}\u0000${absolutePath}`;
|
||||
}
|
||||
@@ -1508,10 +1519,9 @@ function resolveFinalizeErrorMessage(error: unknown): string {
|
||||
if (isBookMetadataConstraintViolation(error)) {
|
||||
return INVALID_METADATA_MESSAGE;
|
||||
}
|
||||
// A database error is not a sentence for a requester to read. One shipped verbatim - a whole
|
||||
// failed SELECT, table and column names included - into the request drawer of the person who
|
||||
// asked for the book. The detail belongs in the log, which already has the original error.
|
||||
if (isDatabaseError(error)) {
|
||||
// Infrastructure errors can contain SQL and absolute filesystem paths. They belong in the
|
||||
// sanitized server log, not in the request drawer of the person who asked for the book.
|
||||
if (isDatabaseError(error) || hasNodeStyleErrorCode(error)) {
|
||||
return INTERNAL_FAILURE_MESSAGE;
|
||||
}
|
||||
if (error instanceof Error && error.message) {
|
||||
@@ -1521,16 +1531,36 @@ function resolveFinalizeErrorMessage(error: unknown): string {
|
||||
}
|
||||
|
||||
/**
|
||||
* A `pg` error carries a five-character SQLSTATE. Drizzle wraps it in a `DrizzleQueryError` whose
|
||||
* own message is the failed query, so the chain is walked rather than the outermost error read.
|
||||
* Drizzle wraps the driver's `DatabaseError` in a `DrizzleQueryError`, so the chain is walked
|
||||
* rather than relying on the outermost error. Checking the concrete error type avoids mistaking
|
||||
* five-character Node codes such as `EPERM` and `EXDEV` for SQLSTATE values.
|
||||
*/
|
||||
function isDatabaseError(error: unknown): boolean {
|
||||
for (const entry of iterateErrorChain(error)) {
|
||||
if (/^[0-9A-Z]{5}$/.test(asString(entry.code))) return true;
|
||||
if (entry instanceof DatabaseError) return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
function hasNodeStyleErrorCode(error: unknown): boolean {
|
||||
for (const entry of iterateErrorChain(error)) {
|
||||
if (/^E[A-Z0-9_]+$/.test(asString(entry.code))) return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
function finalizeErrorLogDetails(error: unknown): { errorClass: string; errorCode: string; message: string } {
|
||||
let selected: Record<string, unknown> | undefined;
|
||||
for (const entry of iterateErrorChain(error)) selected = entry;
|
||||
|
||||
const errorCode = asString(selected?.code) || 'none';
|
||||
const errorClass =
|
||||
selected instanceof Error ? selected.constructor.name : asString(selected?.name) || (error instanceof Error ? error.constructor.name : 'Error');
|
||||
const message = hasNodeStyleErrorCode(error) ? errorCode : asString(selected?.message) || (error instanceof Error ? error.message : String(error));
|
||||
|
||||
return { errorClass, errorCode, message };
|
||||
}
|
||||
|
||||
function isPublishedYearConstraintViolation(error: unknown): boolean {
|
||||
for (const entry of iterateErrorChain(error)) {
|
||||
if (entry.code === '23514') {
|
||||
|
||||
Reference in New Issue
Block a user