Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions src/domain/graph/resolve.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import fs from 'node:fs';
import path from 'node:path';
import { debug } from '../../infrastructure/logger.js';
import { loadNative } from '../../infrastructure/native.js';
import { normalizePath } from '../../shared/constants.js';
import type { BareSpecifier, BatchResolvedMap, ImportBatchItem, PathAliases } from '../../types.js';
Expand Down Expand Up @@ -64,7 +65,10 @@ function getPackageExports(packageDir: string): any {
const exports = pkg.exports ?? null;
_exportsCache.set(packageDir, exports);
return exports;
} catch {
} catch (e) {
debug(
`readPackageExports: failed to read package.json in ${packageDir}: ${(e as Error).message}`,
);
_exportsCache.set(packageDir, null);
return null;
}
Expand Down Expand Up @@ -515,8 +519,10 @@ export function resolveImportPath(
// unresolved ".." components (PathBuf::components().collect() doesn't
// collapse parent refs). Apply the remap on the JS side as a fallback.
return remapJsToTs(normalized, rootDir);
} catch {
// fall through to JS
} catch (e) {
debug(
`resolveImportPath: native resolution failed, falling back to JS: ${(e as Error).message}`,
);
}
}
return resolveImportPathJS(fromFile, importSource, rootDir, aliases);
Expand All @@ -535,8 +541,10 @@ export function computeConfidence(
if (native) {
try {
return native.computeConfidence(callerFile, targetFile, importedFrom || null);
} catch {
// fall through to JS
} catch (e) {
debug(
`computeConfidence: native computation failed, falling back to JS: ${(e as Error).message}`,
);
}
}
return computeConfidenceJS(callerFile, targetFile, importedFrom);
Expand Down Expand Up @@ -575,7 +583,8 @@ export function resolveImportsBatch(
map.set(`${r.fromFile}|${r.importSource}`, resolved);
}
return map;
} catch {
} catch (e) {
debug(`batchResolve: native batch resolution failed: ${(e as Error).message}`);
return null;
}
}
Expand Down
15 changes: 10 additions & 5 deletions src/domain/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,8 @@ async function backfillTypeMap(
if (!code) {
try {
code = fs.readFileSync(filePath, 'utf-8');
} catch {
} catch (e) {
debug(`backfillTypeMap: failed to read ${filePath}: ${(e as Error).message}`);
return { typeMap: new Map(), backfilled: false };
}
}
Expand All @@ -458,7 +459,9 @@ async function backfillTypeMap(
if (extracted?.tree && typeof extracted.tree.delete === 'function') {
try {
extracted.tree.delete();
} catch {}
} catch (e) {
debug(`backfillTypeMap: WASM tree cleanup failed: ${(e as Error).message}`);
}
}
}
}
Expand Down Expand Up @@ -571,14 +574,16 @@ export async function parseFilesAuto(
symbols.typeMap = extracted.symbols.typeMap;
symbols._typeMapBackfilled = true;
}
} catch {
/* skip — typeMap is a best-effort backfill */
} catch (e) {
debug(`batchExtract: typeMap backfill failed: ${(e as Error).message}`);
} finally {
// Free the WASM tree to prevent memory accumulation across repeated builds
if (extracted?.tree && typeof extracted.tree.delete === 'function') {
try {
extracted.tree.delete();
} catch {}
} catch (e) {
debug(`batchExtract: WASM tree cleanup failed: ${(e as Error).message}`);
}
}
}
}
Expand Down
18 changes: 10 additions & 8 deletions src/infrastructure/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -310,8 +310,10 @@ function resolveWorkspaceEntry(pkgDir: string): string | null {
const candidate = path.resolve(pkgDir, idx);
if (fs.existsSync(candidate)) return candidate;
}
} catch {
/* ignore */
} catch (e) {
debug(
`resolveWorkspaceEntry: package.json probe failed for ${pkgDir}: ${(e as Error).message}`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 (e as Error).message unsafe for non-Error throws

The pattern (e as Error).message is used consistently across all 4 changed files. In TypeScript strict mode, e is unknown, and if anything other than an Error instance is thrown (e.g. a plain string, null, or a non-standard exception from the WASM layer), .message will be undefined, producing unhelpful log output like readPackageExports: failed to read package.json in /path: undefined.

A safer, idiomatic alternative:

Suggested change
`resolveWorkspaceEntry: package.json probe failed for ${pkgDir}: ${(e as Error).message}`,
`resolveWorkspaceEntry: package.json probe failed for ${pkgDir}: ${e instanceof Error ? e.message : String(e)}`,

The same pattern appears in all new debug() calls across these files:

  • src/infrastructure/config.ts:315, 349, 367
  • src/infrastructure/native.ts:31, 98
  • src/domain/graph/resolve.ts:70, 522, 544, 585
  • src/domain/parser.ts:443, 462, 576, 582

For the Node.js built-ins (fs, JSON.parse, require) these always throw Error instances, so the risk is low in practice — but the WASM tree.delete() calls could theoretically throw a non-Error value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in follow-up PR #630 — all 14 occurrences of \ in this PR's catch blocks have been replaced with \ for safe handling of non-Error throws.

);
}
return null;
}
Expand Down Expand Up @@ -344,8 +346,8 @@ export function detectWorkspaces(rootDir: string): Map<string, WorkspaceEntry> {
}
}
}
} catch {
/* ignore */
} catch (e) {
debug(`detectWorkspaces: failed to parse pnpm-workspace.yaml: ${(e as Error).message}`);
}
}

Expand All @@ -363,8 +365,8 @@ export function detectWorkspaces(rootDir: string): Map<string, WorkspaceEntry> {
// Yarn classic format: { packages: [...], nohoist: [...] }
patterns.push(...ws.packages);
}
} catch {
/* ignore */
} catch (e) {
debug(`detectWorkspaces: failed to parse package.json workspaces: ${(e as Error).message}`);
}
}
}
Expand All @@ -379,8 +381,8 @@ export function detectWorkspaces(rootDir: string): Map<string, WorkspaceEntry> {
if (Array.isArray(lerna.packages)) {
patterns.push(...lerna.packages);
}
} catch {
/* ignore */
} catch (e) {
debug(`detectWorkspaces: failed to parse lerna.json: ${(e as Error).message}`);
}
}
}
Expand Down
10 changes: 8 additions & 2 deletions src/infrastructure/native.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { createRequire } from 'node:module';
import os from 'node:os';
import { EngineError } from '../shared/errors.js';
import type { NativeAddon } from '../types.js';
import { debug } from './logger.js';

let _cached: NativeAddon | null | undefined; // undefined = not yet tried, null = failed, NativeAddon = module
let _loadError: Error | null = null;
Expand All @@ -26,7 +27,9 @@ function detectLibc(): 'gnu' | 'musl' {
if (files.some((f: string) => f.startsWith('ld-musl-') && f.endsWith('.so.1'))) {
return 'musl';
}
} catch {}
} catch (e) {
debug(`detectLibc: failed to read /lib: ${(e as Error).message}`);
}
return 'gnu';
}

Expand Down Expand Up @@ -92,7 +95,10 @@ export function getNativePackageVersion(): string | null {
try {
const pkgJson = _require(`${pkg}/package.json`) as { version?: string };
return pkgJson.version || null;
} catch {
} catch (e) {
debug(
`getNativePackageVersion: failed to read package.json for ${pkg}: ${(e as Error).message}`,
);
return null;
}
}
Expand Down
Loading