🙏Thoughts & Prayers

TODO: Implement Error Handling

TL;DR: Regex-driven file parser with empty catch blocks and no validation becomes a production disaster.

#typescript#code-smells#god-class#regex#error-handling

The Code

typescript
1// ... (imports and class setup omitted)
2
3private static async process(include: string, tasks: object, task: Task) {
4    if (!Todo.config.excludes[include]) {
5        let files = fs.readdirSync(include, { withFileTypes: true });
6        
7        for (let f=files.length-1; f>=0; f--) {
8            let name = `${include}/${files[f].name}`;
9
10            if (!Todo.config.excludes[name]) {
11                try {
12                    let extension = files[f].name.toString().toLowerCase().split('.');
13                    
14                    if (files[f].isFile() && (Todo.config.extensions['*'] || Todo.config.extensions[extension[extension.length-1]])) {
15                        let file = fs.readFileSync(name).toString();
16                        
17                        let keywords = Todo.config.keywords.join('|');
18                        let match = new RegExp('(#|--|/[/|\\*])((?!\\*/).)*('+keywords+')((?!\\*/).)+', 'ig');
19                        let replace = new RegExp('(#|--|/[/|\\*])((?!\\*/).)*('+keywords+')', 'i');
20                        
21                        let todos = file.match(match); //todo implement matchAll
22                        for (let t=0; t<todos.length; t++) {
23                            let todo = todos[t].replace(replace, '').trim();
24                            todo = `${todo.charAt(0).toUpperCase()}${todo.slice(1)} in ` + `${include}/${files[f].name}`.replace('./', '');
25
26                            if (!tasks[todo] && !tasks[Todo.metaphone(todo)]) {
27                                if (await task.insert(todo)) {
28                                    tasks[Todo.metaphone(todo)] = true;
29                                    console.log(`( Created ) ${todo}`);
30                                }
31                            }
32                            else console.log(`(Duplicate) ${todo}`);
33                        }
34                    }
35                    else if (files[f].isDirectory()) {
36                        await Todo.process(name, tasks, task);
37                    }
38                }
39                catch (error) {
40                    //console.log(error)
41                }
42            }
43        }
44    }
45}
46
47public static metaphone(todo: string) {
48    let extension = todo.split('.');
49    
50    if (extension.length > 1)
51        todo = todo.replace(new RegExp('\\.'+extension[extension.length-1]+'$', 'i'), '');
52    
53    return `${metaphone(todo)}${todo.replace(/[^0-9]/g, '')}`;
54}
55
56// ... (additional methods omitted)
57
58*(Excerpt from larger file - showing relevant section only)*
59
60

The Prayer 🙏

🙏 Please let this magical TODO parser work flawlessly across three different task management systems, ignore all error handling best practices, and somehow make sense of this regex-driven file processing nightmare that definitely won't explode when it encounters unexpected input.

The Reality Check

This code is a production incident waiting to happen. The complete absence of error handling means any malformed JSON, network timeout, or filesystem permission issue will crash the entire application. The regex patterns for parsing TODO comments are fragile and will break on edge cases like nested comments or unusual formatting.

The file processing logic recursively crawls directories without any safeguards against symbolic links, deeply nested structures, or large files that could consume massive amounts of memory. The metaphone-based duplicate detection is clever but unreliable - similar-sounding but different tasks will be incorrectly flagged as duplicates, while actual duplicates with different phonetic signatures will slip through.

The mixed promise/callback patterns and synchronous file operations will block the event loop, making the application unresponsive. In a production environment, this tool would likely fail silently on many codebases, miss legitimate TODOs, create duplicate tickets, and potentially crash when encountering any unexpected file system structure.

The Fix

The first priority is adding comprehensive error handling and input validation. Every file operation and external API call needs proper try-catch blocks with meaningful error messages:

typescript
1private static async processFile(filePath: string): Promise<string[]> {
2  try {
3    const content = await fs.promises.readFile(filePath, 'utf8');
4    return this.extractTodos(content, filePath);
5  } catch (error) {
6    console.warn(`Failed to process ${filePath}: ${error.message}`);
7    return [];
8  }
9}
10

Replace the fragile regex parsing with a proper tokenizer or AST-based approach. For quick wins, make the regex more robust and testable:

typescript
1private static extractTodos(content: string, filePath: string): string[] {
2  const commentPatterns = [
3    /\/\/\s*(TODO|FIXME|HACK)\s*:?\s*(.+)$/gm,
4    /\/\*\s*(TODO|FIXME|HACK)\s*:?\s*([^*]*)\*\//g,
5    /#\s*(TODO|FIXME|HACK)\s*:?\s*(.+)$/gm
6  ];
7  
8  const todos: string[] = [];
9  commentPatterns.forEach(pattern => {
10    let match;
11    while ((match = pattern.exec(content)) !== null) {
12      todos.push(this.formatTodo(match[2].trim(), filePath, match[1]));
13    }
14  });
15  
16  return todos;
17}
18

Implement proper async file operations and add circuit breakers for directory traversal:

typescript
1private static async processDirectory(
2  dirPath: string, 
3  maxDepth: number = 10
4): Promise<void> {
5  if (maxDepth <= 0) {
6    console.warn(`Max depth reached for ${dirPath}`);
7    return;
8  }
9  
10  try {
11    const entries = await fs.promises.readdir(dirPath, { withFileTypes: true });
12    const promises = entries.map(async (entry) => {
13      const fullPath = path.join(dirPath, entry.name);
14      if (this.shouldExclude(fullPath)) return;
15      
16      if (entry.isFile()) {
17        return this.processFile(fullPath);
18      } else if (entry.isDirectory()) {
19        return this.processDirectory(fullPath, maxDepth - 1);
20      }
21    });
22    
23    await Promise.allSettled(promises);
24  } catch (error) {
25    console.error(`Failed to process directory ${dirPath}: ${error.message}`);
26  }
27}
28

Replace the metaphone duplicate detection with a proper hash-based system and add configuration validation:

typescript
1private static generateTodoHash(content: string, filePath: string): string {
2  return crypto.createHash('md5')
3    .update(`${content.toLowerCase().trim()}:${filePath}`)
4    .digest('hex');
5}
6

Lesson Learned

Error handling isn't optional decoration - it's the difference between a useful tool and a production disaster.