Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces three new TypeScript scripts (gasbill.ts, pcmc.ts, and rrsp.ts) to parse financial PDF statements (gas bills, PC MasterCard, and Manulife RRSP) and extract relevant information using pdf-parse and commander. It also makes a minor logging adjustment in mspdf.ts. The review feedback highlights potential resource leaks across all three new scripts because parser.destroy() is not guaranteed to run if an error occurs during PDF parsing; wrapping these calls in try...finally blocks is recommended. Additionally, parsePdfAndExtractInfo in gasbill.ts is missing a try...catch block for handling extraction errors, unlike the other scripts.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| async function parsePdfAndPrint(filePath: string): Promise<void> { | ||
| if (!fs.existsSync(filePath)) { | ||
| console.error(`Error: File not found at path "${filePath}"`); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| const dataBuffer = fs.readFileSync(filePath); | ||
| const parser = new PDFParse({ data: dataBuffer }); | ||
| const result = await parser.getText(); | ||
| console.log(result.text); | ||
| await parser.destroy(); | ||
| } |
There was a problem hiding this comment.
If parser.getText() throws an error, parser.destroy() will not be called, potentially leaking resources. Wrap the parsing logic in a try...finally block to ensure parser.destroy() is always executed.
async function parsePdfAndPrint(filePath: string): Promise<void> {
if (!fs.existsSync(filePath)) {
console.error(`Error: File not found at path "${filePath}"`);
process.exit(1);
}
const dataBuffer = fs.readFileSync(filePath);
const parser = new PDFParse({ data: dataBuffer });
try {
const result = await parser.getText();
console.log(result.text);
} finally {
await parser.destroy();
}
}| async function extractInfoFromPdf(filePath: string): Promise<ParsedGasBill> { | ||
| const dataBuffer = fs.readFileSync(filePath); | ||
| const parser = new PDFParse({ data: dataBuffer }); | ||
| const result = await parser.getText(); | ||
| await parser.destroy(); |
There was a problem hiding this comment.
If parser.getText() throws an error, parser.destroy() will not be called. Use a try...finally block to guarantee resource cleanup.
| async function extractInfoFromPdf(filePath: string): Promise<ParsedGasBill> { | |
| const dataBuffer = fs.readFileSync(filePath); | |
| const parser = new PDFParse({ data: dataBuffer }); | |
| const result = await parser.getText(); | |
| await parser.destroy(); | |
| async function extractInfoFromPdf(filePath: string): Promise<ParsedGasBill> { | |
| const dataBuffer = fs.readFileSync(filePath); | |
| const parser = new PDFParse({ data: dataBuffer }); | |
| let result; | |
| try { | |
| result = await parser.getText(); | |
| } finally { | |
| await parser.destroy(); | |
| } |
| async function parsePdfAndExtractInfo(filePath: string): Promise<void> { | ||
| if (!fs.existsSync(filePath)) { | ||
| console.error(`Error: File not found at path "${filePath}"`); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| const parsed = await extractInfoFromPdf(filePath); | ||
|
|
||
| console.log(`Pre-authorized Withdrawal: ${parsed.preAuthorizedWithdrawal}`); | ||
| console.log(`Issue Date: ${parsed.issueDate}`); | ||
| } |
There was a problem hiding this comment.
Unlike pcmc.ts and rrsp.ts, this function lacks a try...catch block around extractInfoFromPdf. If parsing fails, it will result in an unhandled promise rejection. Add error handling to gracefully log the error and exit.
| async function parsePdfAndExtractInfo(filePath: string): Promise<void> { | |
| if (!fs.existsSync(filePath)) { | |
| console.error(`Error: File not found at path "${filePath}"`); | |
| process.exit(1); | |
| } | |
| const parsed = await extractInfoFromPdf(filePath); | |
| console.log(`Pre-authorized Withdrawal: ${parsed.preAuthorizedWithdrawal}`); | |
| console.log(`Issue Date: ${parsed.issueDate}`); | |
| } | |
| async function parsePdfAndExtractInfo(filePath: string): Promise<void> { | |
| if (!fs.existsSync(filePath)) { | |
| console.error(`Error: File not found at path "${filePath}"`); | |
| process.exit(1); | |
| } | |
| try { | |
| const parsed = await extractInfoFromPdf(filePath); | |
| console.log(`Pre-authorized Withdrawal: ${parsed.preAuthorizedWithdrawal}`); | |
| console.log(`Issue Date: ${parsed.issueDate}`); | |
| } catch (error) { | |
| const message = error instanceof Error ? error.message : String(error); | |
| console.error(`Error: ${message}`); | |
| process.exit(1); | |
| } | |
| } |
| async function parsePdfAndPrint(filePath: string): Promise<void> { | ||
| if (!fs.existsSync(filePath)) { | ||
| console.error(`Error: File not found at path "${filePath}"`); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| const dataBuffer = fs.readFileSync(filePath); | ||
| const parser = new PDFParse({ data: dataBuffer }); | ||
| const result = await parser.getText(); | ||
| console.log(result.text); | ||
| await parser.destroy(); | ||
| } |
There was a problem hiding this comment.
If parser.getText() throws an error, parser.destroy() will not be called, potentially leaking resources. Wrap the parsing logic in a try...finally block to ensure parser.destroy() is always executed.
async function parsePdfAndPrint(filePath: string): Promise<void> {
if (!fs.existsSync(filePath)) {
console.error(`Error: File not found at path "${filePath}"`);
process.exit(1);
}
const dataBuffer = fs.readFileSync(filePath);
const parser = new PDFParse({ data: dataBuffer });
try {
const result = await parser.getText();
console.log(result.text);
} finally {
await parser.destroy();
}
}| async function extractInfoFromPdf(filePath: string): Promise<ParsedPcmc> { | ||
| const dataBuffer = fs.readFileSync(filePath); | ||
| const parser = new PDFParse({ data: dataBuffer }); | ||
| const result = await parser.getText(); | ||
| await parser.destroy(); |
There was a problem hiding this comment.
If parser.getText() throws an error, parser.destroy() will not be called. Use a try...finally block to guarantee resource cleanup.
| async function extractInfoFromPdf(filePath: string): Promise<ParsedPcmc> { | |
| const dataBuffer = fs.readFileSync(filePath); | |
| const parser = new PDFParse({ data: dataBuffer }); | |
| const result = await parser.getText(); | |
| await parser.destroy(); | |
| async function extractInfoFromPdf(filePath: string): Promise<ParsedPcmc> { | |
| const dataBuffer = fs.readFileSync(filePath); | |
| const parser = new PDFParse({ data: dataBuffer }); | |
| let result; | |
| try { | |
| result = await parser.getText(); | |
| } finally { | |
| await parser.destroy(); | |
| } |
| async function parsePdfAndPrint(filePath: string): Promise<void> { | ||
| if (!fs.existsSync(filePath)) { | ||
| console.error(`Error: File not found at path "${filePath}"`); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| const dataBuffer = fs.readFileSync(filePath); | ||
| const parser = new PDFParse({ data: dataBuffer }); | ||
| const result = await parser.getText(); | ||
| console.log(result.text); | ||
| await parser.destroy(); | ||
| } |
There was a problem hiding this comment.
If parser.getText() throws an error, parser.destroy() will not be called, potentially leaking resources. Wrap the parsing logic in a try...finally block to ensure parser.destroy() is always executed.
async function parsePdfAndPrint(filePath: string): Promise<void> {
if (!fs.existsSync(filePath)) {
console.error(`Error: File not found at path "${filePath}"`);
process.exit(1);
}
const dataBuffer = fs.readFileSync(filePath);
const parser = new PDFParse({ data: dataBuffer });
try {
const result = await parser.getText();
console.log(result.text);
} finally {
await parser.destroy();
}
}| async function extractInfoFromPdf(filePath: string): Promise<ParsedRrsp> { | ||
| const dataBuffer = fs.readFileSync(filePath); | ||
| const parser = new PDFParse({ data: dataBuffer }); | ||
| const result = await parser.getText(); | ||
| await parser.destroy(); |
There was a problem hiding this comment.
If parser.getText() throws an error, parser.destroy() will not be called. Use a try...finally block to guarantee resource cleanup.
| async function extractInfoFromPdf(filePath: string): Promise<ParsedRrsp> { | |
| const dataBuffer = fs.readFileSync(filePath); | |
| const parser = new PDFParse({ data: dataBuffer }); | |
| const result = await parser.getText(); | |
| await parser.destroy(); | |
| async function extractInfoFromPdf(filePath: string): Promise<ParsedRrsp> { | |
| const dataBuffer = fs.readFileSync(filePath); | |
| const parser = new PDFParse({ data: dataBuffer }); | |
| let result; | |
| try { | |
| result = await parser.getText(); | |
| } finally { | |
| await parser.destroy(); | |
| } |
No description provided.