Clean up exception handling in the core
Created by: tsodring
Exception handling is something that wasn't really well thought out and was added on a ad-hoc basis. We should tidy up this.
There are two NikitaException classes defined in the code:
- core-common: package nikita.util.exceptions;
- core-webapp: package no.arkivlab.hioa.nikita.webapp.util.exceptions;
I'm not even sure why this happened. There is a thinking that the code in core-common is used by core-webapp and core-extraction and perhaps some other modules later. It's a little unclear if e.g. REST exceptions should be defined in core-common or core-webapp. Although I do think that someone might create a different type of REST service on top of core-common so it might be worthwhile to just have all exceptions defined in core-common.
There is also a notion of NikitaException and NoarkException. The thinking here was that NikitaExceptions are application specific, while NoarkExceptions are related to Noark structure problems etc. The two kinda just merged into the one concept as the codebase progressed. I'm not sure if we should distinguish between the two or just have NikitaExceptions. Exception handling is not defined in the interface standard, just HTTP status codes so we can decide what we want to do.
I do think that we should have NikitaException as the base class, with perhaps some subclasses for Noark exceptions and perhaps other sub-classes for e.g REST exceptions. I'm subclassing them so I can capture exceptions in a ResponseEntityExceptionHandler.
You can see exceptions being caught in RestResponseEntityExceptionHandler in core-webapp: no.arkivlab.hioa.nikita.webapp.util.error;
The thinking for doing it this way is to allow us have more finer grained control in dealing with exceptions, perhaps we have to change the HTTP status code in a particular scenario or the interface standard later specifies a particular JSON format for the response for exceptional events.
I think we're on the right track, but need to clean up the code a little and remove the duplicate NikitaException. Exceptions should be thrown with a decent message. We should use 3 levels,
- A simple description
- A more detailed description of what went wrong
- A developer message perhaps including relevant portion of a stack trace if relevant
This allows the client to show the appropriate level information to the user.