2

I want a non-primary key column to be unique. This is enforced with a unique constraint. In a service endpoint I want to return a nice error message structured into a {field, messsage} object when unique constraints are violated. Currently I check if the column is unique in the service layer and throw a UniqueFieldExcepton to be handled globally.

Other options I have considered are:

  1. Handling the check in the database layer (repository, DAL, etc) and throwing the UniqueFieldExcepton to be handled globally.

  2. Catching the database error in the database layer, parsing the column name from the message and throwing a UniqueFieldExcepton to be handled globally.

  3. Catching the database error in the service layer, trying to parse the column name from the message and throwing a UniqueFieldExcepton to be handled globally.

My team has different opinions on how to solve this.

Are there any guidelines/best practices that could help us to come to an agreement?

walpen
  • 3,231
Jakob
  • 213

2 Answers2

1

This is fairly subjective, but I like error handling to be as generic and reusable as possible. To that end, I would have a data layer error handling (especially nice if all you database access goes through a common facade) that handles the error and parsing database exceptions into your application's exceptions.

Then at a global/service layer if you needed you could have a translator to go from application exceptions to service exceptions if necessary.

Daniel Bickler
  • 111
  • The problem with this approach is that different database drivers could theoretically throw different exceptions. You would have to implement it for each implementation of the database interface. Also this is harder to test. This seems a bit fragile but the benefit is reuse of the exceptions that could be used between multiple entities. This might just be opinionated... – Jakob Apr 04 '17 at 08:27
  • It is databas-specific. But unless you query the database anything you do will be database specific. How likely is it that you will change databases? – Daniel Bickler Apr 04 '17 at 12:08
  • I support 5 different types of databases through java odbc. Parsing the errors feels fragile, I would have to parse the column name from a errorMessage which has a different name from the entity. – Jakob Apr 04 '17 at 12:40
  • Right. If you don't want to do that (and it's not ideal) then making a separate query ala 'a' is your best bet. You just have to be willing to take the performance hit of making another query for every insert. I'd be interested if others have better ideas because I've always wondered how others solve this. – Daniel Bickler Apr 04 '17 at 13:21
  • 1
    I think if you send a command to the database and it returns an error indicating the duplicate, shouldn't your code already know what the Unique Value is and shouldn't have to parse that from the error message? – JeffO Apr 04 '17 at 15:46
  • @JeffO you could handle the error code and just hope no one adds another unique column to the table. Also what I have seen some frameworks throw different exceptions between add and update so this seems very fragile to handle. Also the orm framework could change it's exceptions... – Jakob Apr 04 '17 at 18:46
  • 1
    @Jakob - I would also handle this in the business logic code. Multiple unique columns are probably derived from business rules. The database error returned shouldn't be the only check, but is necessary in case data is entered outside the application. – JeffO Apr 06 '17 at 16:37
  • +1 JeffO. Db constraints are the last line of the defense, not the only. – Laiv Apr 06 '17 at 21:04
0

The problem with all your three approaches is the severe code duplication it would lead to. The first one appears particularly scary to me:

  • When you're inserting a product, the DAL checks if the product name already exists. If yes, throw an exception. Then it checks if SKU exists. Yes? Throw an exception. What about EAN-13? Throw an exception!

  • Now you're inserting a customer. Same story: multiple checks, which eventually result in an exception.

  • What about a product review? Same thing here.

The second and third approaches are slightly better, since you don't need to check manually for every UNIQUE field. Just do the query, catch the exception of the underlying third-party library, and throw your own exception instead. Still, you'll duplicate this logic in a method which inserts a product, and the one which creates a customer, and the one which adds a product review. Not good.

Another issue is that by catching and throwing your own exception instead of catching-and-rethrowing (or simply letting the exception to go up the stack) is that you're breaking the stack. It creates debugging nightmares.¹

In general, you catch exceptions when you can handle them. For instance, you catch an exception which tells that the file doesn't exist in a method which is able to decide whether it should substitute the contents of the file by default contents, or go search for contents somewhere else—such as a backup location—or ask user for help. So in which layer should we catch exceptions related to UNIQUE constraint violation? Probably in UI layer: for some reason (for instance, performance) the user interface was unable to prevent the user from typing a duplicate information in the first place, submitted the input downstream, and caused the constraint violation. It's also the UI which can actually handle the exception, for example by telling the user that the entered value is already in use.

The problem with this approach is that different database drivers could theoretically throw different exceptions. You would have to implement it for each implementation of the database interface. [...] I support 5 different types of databases through java odbc.

This is not a problem.

UI catches the exception which comes right from the third-party library and so looks more like java.sql.SQLIntegrityConstraintViolationException rather than myapp.UniqueFieldExcepton. But UI doesn't know how to parse the message within the exception in order to extract the name of the concerned field. The DAL knows it.

So UI does a call to BL, passing the exception message and asking which field is concerned. BL propagates the request down the DAL. The DAL parses the message and returns the field ID. Now, UI can highlight the corresponding field and show a meaningful error.

¹ I'm not even talking figuratively here. I still remember how painful it was to work with legacy code I had to support six years ago. One of the WTFs was that developers loved catching exceptions in places where they couldn't possibly handle them anyway. So sometimes, they were rethrowing their own. Sometimes, they were throwing the same exception, but doing it wrongly and losing the stack trace. And sometimes, they would just swallow those exceptions and pretend nothing wrong happened, reinventing On Error Resume Next. A SELECT query failed? That's OK, we'll still loop on the sequence it expected to return to us. What do you mean by “NullPointerException”?

Community
  • 1
Arseni Mourzenko
  • 135,365
  • I'm developing a rest service, I don´t want to expose database exceptions to the rest user. Also the database exceptions I have seen do not indicate what field was not unique just that some field was not unique so the front end would have to quess which filed was duplicated. – Jakob May 07 '17 at 00:48
  • @Jakob So your option 2 consist of parsing the column name from a message without a column name? – Stop harming Monica Jun 06 '17 at 11:25