nestml icon indicating copy to clipboard operation
nestml copied to clipboard

Halt on first error by default

Open clinssen opened this issue 4 years ago • 5 comments

Sometimes it can be hard to trace back an error in the log, because processing deliberately tries to continue in spite of errors, which means many more log messages tend to obscure the initial error statement.

I propose to make halting on the first error the default, and to include an override (i.e., replicating the current behaviour) under the existing --dev parameter.

clinssen avatar Apr 06 '20 12:04 clinssen

I agree that continuation in case of errors should be enabled on will and not be the default. However, maybe it would be better if a separate flag (e.g. --continue-with-errors) was added?

jougs avatar Apr 14 '20 10:04 jougs

@Silmathoron: @jougs and I discussed dropping the option to continue in spite of errors altogether. We found it difficult to come up with a use case, where you'd want to generate code for multiple models, but not mind one or more of the models failing. In the case of generating code for a single model, you typically just look at the first error, fix it, and then re-invoke NESTML, because the subsequent error messages are likely to have change substantially. Do you have any objections?

clinssen avatar Apr 14 '20 14:04 clinssen

Additionally, we suggest to rename the part of --dev that adds Jinja2 debugging annotations in the generated code to --annotate_target_code. --dev would then disappear completely.

clinssen avatar Apr 14 '20 14:04 clinssen

No objections, this makes complete sense to me: halt on first error by default and remove --dev :+1:

Silmathoron avatar Apr 14 '20 16:04 Silmathoron

Further notes:

  • This affects many unit tests, which check the number of error messages generated during processing. These would be reduced to a "suceed or fail" test.

  • I am looking for the best place to raise exceptions. Mostly, NESTML tries its hardest to continue in spite of errors, so you would tend to see the pattern

    if some_error_occurred:
        Logger.log_message(..., log_level=LoggingLevel.ERROR)
        return
    

    We could systematically replace the return statement with raising an exception, catch it in ModelParser.parse_model(), and print an additional error message ("errors occurred during processing: abort").

clinssen avatar Apr 16 '20 09:04 clinssen