FLINT icon indicating copy to clipboard operation
FLINT copied to clipboard

Make our use of Exceptions consistent

Open Patamap opened this issue 5 years ago • 7 comments

Make our use of Exceptions consistent - for example remove any boost exceptions handling at higher levels. Any special exception handling should be caught at lower levels, and moja exceptions passed on. The system should use it’s own collection of std exceptions.

Please feel free to contact Jim via Slack, https://app.slack.com/client/T1G1M5HPF/D014VTVKJEA for further questions/comments.

Patamap avatar Jun 09 '20 08:06 Patamap

Hello @Patamap I would like to work on this issue, so can you please elaborate what I need to do to fix this issue?

AlkaDas991 avatar Mar 21 '21 05:03 AlkaDas991

@patamap can you please explain me a little bit about the issue

AlkaDas991 avatar Mar 23 '21 05:03 AlkaDas991

@mfellows - could you please add a brief description to help @AlkaDas991? I know exception handling was one of your bugbears.

aornugent avatar Apr 06 '21 23:04 aornugent

Hi Alka, thanks for your interest in this issue -

One example would be spatialtiledlocaldomaincontroller.cpp lines 794-840, you can see the huge variety of exceptions in there - which in theory should be helpful to narrow errors down, but in reality you need a try/catch block a mile long to extract all the information out of the different exception types.

This is particularly a problem in user (module) code where instead of catching all the different exception types in flintexceptions.h, we (well, I...) lazily catch FLINTException, and then all those boost::error_info details get lost. I think it would tidy things up a lot if we either got rid of all the specialized exceptions and stuck with a single one like SimulationError, or required FLINTException subclasses to override what() or some other method that rolls up all the details into some user or developer-friendly text.

There's a fair amount of flexibility for fixing this issue, steps would basically be:

  • have a look through the code (i.e. spatialtiledlocaldomaincontroller) and notice how ugly the exception stuff currently is
  • propose a strategy for improving the situation: could be one of my suggestions above or something completely different, but probably best to let people have a look before investing effort into coding
  • implement whatever solution you come up with

mfellows avatar Apr 09 '21 18:04 mfellows

Hey @mfellows @aornugent Can I be assigned this issue, it has been inactive for a while and I think I can implement my solution here.

SlipperyGnome avatar Apr 15 '21 06:04 SlipperyGnome

That's fine - could you please also coordinate with @AlkaDas991 so that you can both learn together? I'll connect you on Slack.

aornugent avatar Apr 15 '21 22:04 aornugent

Hi @mfellows, I discussed this with @aornugent and it would be great if you look into this as well. So I have thought of shifting all the boost::get_error_info and _spaciallocationinfo parts of SimulationError and LocalDomainError to flintexception.h that way it'll tidy up the spacialtiledlocaldomaincontroller.cpp file as these try/catch blocks have been used everywhere in that file. This way the exceptions.h file and flintexception.h file will have a similar layout and it'll be easier for a developer to go through this file.

SlipperyGnome avatar Apr 17 '21 09:04 SlipperyGnome