Error in PATCH does not rollback changes

[thinkerbot]

I'm trying to upgrade to 0.4.2 from 0.3.1 and found that an error in a PATCH request will no longer rollback changes to the database made during the update. Something is going on that makes it so the request is not wrapped in a transaction. I'm not sure if other methods are affected as well.

To reproduce I forked peeps and added a bit of code make a change to the database and then raise an error. PhoneNumberResource looks like this afterwards (the code is here).

class PhoneNumberResource < JSONAPI::Resource
  attributes :name, :phone_number
  has_one :contact

  filter :contact

  def name=(value)
    if value == "boom"
      model.name = "change to database that isn't rolled back"
      model.save
      raise "some error"
    end

    model.name = value
  end
end

With this code make a phone number:

curl -i -H "Accept: application/vnd.api+json" -H 'Content-Type:application/vnd.api+json' -X POST -d '{ "data": { "type": "phone-numbers", "relationships": { "contact": { "data": { "type": "contacts", "id": "1" } } }, "attributes": { "name": "home", "phone-number": "(603) 555-1212" } } }' http://localhost:3000/phone-numbers
# ...
# {"data":{"id":"2","type":"phone-numbers","links":{"self":"http://localhost:3000/phone-numbers/2"},"attributes":{"name":"home","phone-number":"(603) 555-1212"},"relationships":{"contact":{"links":{"self":"http://localhost:3000/phone-numbers/2/relationships/contact","related":"http://localhost:3000/phone-numbers/2/contact"},"data":{"type":"contacts","id":"1"}}}}}

Then cause the error:

curl -i -H "Accept: application/vnd.api+json" -H 'Content-Type:application/vnd.api+json' -X PATCH -d '{ "data": { "type": "phone-numbers", "id": "2", "attributes": { "name": "boom" } } }' http://localhost:3000/phone-numbers/2
# HTTP/1.1 500 Internal Server Error
# ...

Then see that the change prior to the error (setting name = "change to database that isn't rolled back") was committed and not rolled back:

curl http://localhost:3000/phone-numbers/2
# {"data":{"id":"2","type":"phone-numbers","links":{"self":"http://localhost:3000/phone-numbers/2"},"attributes":{"name":"change to database that isn't rolled back","phone-number":"(603) 555-1212"},"relationships":{"contact":{"links":{"self":"http://localhost:3000/phone-numbers/2/relationships/contact","related":"http://localhost:3000/phone-numbers/2/contact"},"data":{"type":"contacts","id":"1"}}}}}

[lgebhardt]

That's happening because you are raising an error that isn't derived from JSONAPI::Exceptions::Error. I hadn't noticed that this bypassed the rollback code.

I'll add some handling for other types of errors so the rollback happens. I'll probably return a 500 Internal Server Error in this case and log the exception.

If you wish to use the exception for flow control you will still probably want to create your own OperationsProcessor and catch that type of exception to turn it into a meaningful error for your system.

[thinkerbot]

Great! Thanks. If I understand this right then I like the way you're going to return a JSONAPI error response when an arbitrary error happens during processing. We had added a custom error handler to format arbitrary errors as JSONAPI error responses because the rails default is to print out an html error page (not so ideal). Seems like that will no longer be necessary.

Thanks for the note about flow control -- no flow control in my specific case but I'm glad to know there is a way to do that.

[lgebhardt]

I look at the Internal Server Error as a final line of defense to keeping the responses clean and compliant with JSON API. I think having a custom error handler for any anticipated errors is still the way to go.