concerto icon indicating copy to clipboard operation
concerto copied to clipboard

Improve Error Handling in parseModels Function

Open hussein-saad opened this issue 9 months ago • 1 comments

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

hussein-saad avatar Mar 19 '25 18:03 hussein-saad

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:

  1. Wrap Parser.parse in a try catch block to handle errors per file
  2. For better debugging, file identifiers like modelFile.fileName to error messages can be added
  3. 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 ?

fuyalasmit avatar Mar 20 '25 03:03 fuyalasmit

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.

github-actions[bot] avatar May 20 '25 02:05 github-actions[bot]