Handle errors and warns

[adfarth]

• Converts all @ errors() (which does not stop code) to throw(@ errors()) (which does stop code and logs)
• Collects all logs with log level >= Warn and stores in logfile.log a global logger src/logREopt.jl (events with log level >= @ info printed to console.
• The logger has 2 output modes: a console out to print log messages to console, and a dictionary logREopt.d
• Categorize log messages under "Warn" or "Error" keys in logREopt.d
• Save file and line where the warning or error occurred
• Append all warnings and errors to results dictionary after optimization.

Added

• Add REoptLogger type of global logger with a standard out to the console and to a dictionary
  • Instantiate logREopt as the global logger in __init()__ function call as a global variable
  • Handle Warn or Error logs to save them along with information on where they occurred
  • Try-catch core/reopt.jl -> run_reopt() functions. Process any errors when catching the error.
  • Add Warnings and Errors from logREopt to results dictionary. If error is unhandled in REopt, include a stacktrace
  • Add a status of error to results for consistency
  • Ensure all error text is returned as strings for proper handling in the API
• Add handle_errors(e::E, stacktrace::V) where {E <: Exception, V <: Vector} and handle_errors() to core/reopt.jl to gracefully handle scenarios where REopt fails during data processing/optimization

Changed

• core/reopt.jl added try-catch statements to gracefully catch a REopt error and return it to user along with where the error happened.

TODO:

• Change results["messages"] to Dict; add keys with related input, constraint, or result (e.g. PV input or core/pv.jl) and list of strings with messages (e.g. ["Warning: 1", "Warning: 2"]) handled by logREopt handle_messages()
• Make sure log file properly closes and that new logs overwrite old ones. NA since we are using a dictionary and global logger
• Check multiple runs on development server
• (separate from this PR) Add to API (see Pass REopt.jl errors/warns to API v3 requestor REopt_API#363)

[rathod-b]

Note for reviewer:
Implemented the logREopt functionality using a custom logger since we need a custom sink (dictionary) for warn or error log messages. The sink is returned as part of results dictionary or by itself in case an error is thrown as part of graceful REopt exit.

Graceful exist is only implemented on run_reopt functions which a user/API might call for the time being.

[rathod-b]

Thinking about how this can be implemented in the API, should we simplify the Messages dictionary to just be for Warnings? In case of an error, we can add another Error key. Current implementation might require creating a Messages model and Warnings model within it

[adfarth]

Thinking about how this can be implemented in the API, should we simplify the Messages dictionary to just be for Warnings? In case of an error, we can add another Error key. Current implementation might require creating a Messages model and Warnings model within it

I think avoiding nested dicts is a good idea. I'd be fine with either combining both into a Messages dict (with each message either pre-pended by "Warning:" or "Error:") or separating them out. We definitely want to report back errors though!

[Bill-Becker]

Thinking about how this can be implemented in the API, should we simplify the Messages dictionary to just be for Warnings? In case of an error, we can add another Error key. Current implementation might require creating a Messages model and Warnings model within it

I think avoiding nested dicts is a good idea. I'd be fine with either combining both into a Messages dict (with each message either pre-pended by "Warning:" or "Error:") or separating them out. We definitely want to report back errors though!

One thought, open to debate, is to always have the same keys in the response dictionary, and if there are e.g. no errors, the value/object in the Error key is null. We have a conditional dictionary key within messages for errors in the current API response, and I think that it makes it harder to parse.

[adfarth]

@rathod-b could you update the changelog and description of this PR?

[rathod-b]

Thinking about how this can be implemented in the API, should we simplify the Messages dictionary to just be for Warnings? In case of an error, we can add another Error key. Current implementation might require creating a Messages model and Warnings model within it

I think avoiding nested dicts is a good idea. I'd be fine with either combining both into a Messages dict (with each message either pre-pended by "Warning:" or "Error:") or separating them out. We definitely want to report back errors though!

One thought, open to debate, is to always have the same keys in the response dictionary, and if there are e.g. no errors, the value/object in the Error key is null. We have a conditional dictionary key within messages for errors in the current API response, and I think that it makes it harder to parse.

I agree with prior comments, and will update the output so we always have a Warnings and Errors keys in the results with nothing in the value if there are no warnings or errors.

[rathod-b]

Thinking about how this can be implemented in the API, should we simplify the Messages dictionary to just be for Warnings? In case of an error, we can add another Error key. Current implementation might require creating a Messages model and Warnings model within it

I think avoiding nested dicts is a good idea. I'd be fine with either combining both into a Messages dict (with each message either pre-pended by "Warning:" or "Error:") or separating them out. We definitely want to report back errors though!

One thought, open to debate, is to always have the same keys in the response dictionary, and if there are e.g. no errors, the value/object in the Error key is null. We have a conditional dictionary key within messages for errors in the current API response, and I think that it makes it harder to parse.

I agree with prior comments, and will update the output so we always have a Warnings and Errors keys in the results with nothing in the value if there are no warnings or errors.

Hi @Bill-Becker @adfarth do you guys see any issues with the following return format for Warnings and Errors:

• results["Warnings"] contains all warnings in a vector of tuples as such:
  
• Access where the warning occurred using results["Warnings"][1][1] and access warnings using r["Warnings"][1][2] in Julia
• I am having trouble envisioning what this might look like in the API response? I will try creating a vector output field and see what happens.

Here is the output for a graceful error. I've kept the same format for now. I hope its okay to include the entire stacktrace?

[Bill-Becker]

@rathod-b can you merge this if it's all set, or call out any remaining items to be addressed?

[Bill-Becker]

@adfarth I just asked @rathod-b to merge this if it's all set, but then realized you opened the pull request, so I'm just pinging you too in case you should be the one to do the final check and merge. Thanks!

[rathod-b]

@Bill-Becker I have resolved all but 2 comments where I wasnt sure what the next step should be. One in src/core/heating_cooling_loads.jl and the other in src/core/existing_boiler.jl.

[zolanaj]

I didn't have any comments to address here (I was only referenced in a comment), so I'll append my approval to that of @Bill-Becker

[hdunham]

I'm curious if it's really necessary to test all 4 run_reopt() versions (1 and 2 models, file path and dict) here again if you already did that above?

[rathod-b]

I think its okay to have these tests. Mainly the first 4 tests are looking at invoking REopt.jl in 4 different ways and handling an error which REopt.jl already handles with throw(@error(...)) statement. The last 4 tests do the same thing as first, but the error generated now is an unhandled error where REopt is supposed to return the stack + error message. So these tests just try different ways to test that both iterations of handle_errors() function are working as expected.

I didn't see any considerable impact on solve times since the scenarios are supposed to error out before they begin solving.