Implement suggestion from #58, with test coverage #91
Conversation
| def scimitar_rescuable_exceptions | ||
| [ | ||
| ActiveRecord::RecordInvalid, | ||
| ActiveRecord::RecordNotSaved |
There was a problem hiding this comment.
| ActiveRecord::RecordNotSaved | |
| ActiveRecord::RecordNotSaved, | |
| ActiveRecord::RecordNotUnique |
There was a problem hiding this comment.
This raises the issue of exceptions that do not leave an 'offending' record with validation errors present. AFAICT, RecordNotUnique is a thin adapter-specific wrapper over the DB constraint error and does not know what attribute to mark as errant in the model, so does not add error details.
ActiveRecord's documentation on its exceptions is actually very poor and in almost every case lacks information on the fields present in an exception instance and whether or not there are any alterations present in a record that had an operation performed which led to an exception being generated. That's partly of course down to the circumstances under which the error is raised (it's the validation phase in #save! which updates the record's errors collection, not the subsequent raised exception) but it's still something you need to know as an exception handler. I was left reading source code and eventually decided to give up, in part because source code isn't a contract of behaviour anyway; only API docs cover that. Any undocumented behaviour upon which Scimitar relied could change in future.
All of this means the error details in the SCIM response might be incorrect, but since it's a wider issue for all handled exceptions, I'm going to handle this more robustly and will push an update to the PR soon.
…ndling in conjunction with that request
|
@gsar / @bagp1 - Suggestions from @gsar with related adjustments described by comment above implemented; ready for re-review. |
Implement suggestion from #58, with test coverage.