What I noticed
I was going through runner.ts and noticed that the training entry is imported directly with:
const mod = (await import(pathToFileURL(abs).href)) as Record<
string,
unknown
>;
The missing entry case already has a clear error message, but the import() itself isn't wrapped in any error handling.
So if the entry file exists, but something goes wrong while importing it (for example, one of its imports or dependencies is broken), the original error just bubbles up.
In that case, it may not be immediately obvious that the failure happened while loading the training entry, before the training job actually started.
What could be improved
It could be useful to add some context around the import failure, while still preserving the original error.
For example:
Failed to load training entry: <path>
Cause: <original error>
This would make it easier to understand where the failure happened without hiding the actual underlying error.
Possible approach
- Wrap the training entry
import() in a try/catch
- Add context about which training entry failed to load
- Preserve the original error as the cause
- Add a test covering a failed entry import
I came across this while going through runner.ts and thought this might be worth looking into.
What I noticed
I was going through
runner.tsand noticed that the training entry is imported directly with:The missing entry case already has a clear error message, but the
import()itself isn't wrapped in any error handling.So if the entry file exists, but something goes wrong while importing it (for example, one of its imports or dependencies is broken), the original error just bubbles up.
In that case, it may not be immediately obvious that the failure happened while loading the training entry, before the training job actually started.
What could be improved
It could be useful to add some context around the import failure, while still preserving the original error.
For example:
This would make it easier to understand where the failure happened without hiding the actual underlying error.
Possible approach
import()in atry/catchI came across this while going through runner.ts and thought this might be worth looking into.