Improve Error Handling in parseModels Function
Discussion 🗣
The current implementation of the parseModels function in the parser module lacks error handling. When parsing an array of model files, if one file fails to parse, the entire function throws an exception immediately. This prevents the processing of subsequent files, which could be valid, and makes debugging and maintenance more difficult.
Context
In environments where multiple model files are processed, it's common that one or a few files might contain errors while the others are correctly formatted. Immediate termination upon encountering an error not only halts the overall process but also leaves users without any parsed results from valid files.
Current implementation in parserMain.js:
function parseModels(files, options) {
const result = {
$class: `${MetaModelNamespace}.Models`,
models: [],
};
files.forEach((modelFile) => {
let metaModel = Parser.parse(modelFile, options);
result.models.push(metaModel);
});
return result;
}
Proposed improvements
- Add a way to collect errors rather than throwing immediately
- Include file identifiers in error messages
Hi, I’d like to work on improving the error handling in parseModels.
As mentioned, the function throws an exception on the first error, which stops processing valid files.
Here’s my plan to solve this:
- Wrap Parser.parse in a try catch block to handle errors per file
- For better debugging, file identifiers like modelFile.fileName to error messages can be added
- For collecting all errors without stopping execution, errors array can be returned in the result object
Something like this:
function parseModels(files, options) {
const result = {
$class: `${MetaModelNamespace}.Models`,
models: [],
errors: [] // for collecting errors
};
files.forEach((modelFile) => {
try {
const metaModel = Parser.parse(modelFile, options);
result.models.push(metaModel);
} catch (error) {
// error message with file identifier(file name)
const enhancedError = new Error(`Error parsing "${modelFile.name}": ${error.message}`);
result.errors.push(enhancedError);
}
});
return result;
}
I'd submit a PR if this seems to be good approach.
Can you assign this to me @mttrbrts @sanketshevkar ?
This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 10 days.