invoice.save returns false but invoice.errors is empty

[virajvchavan]

invoice.save returns false but invoice.errors is empty

invoice object is built with some params

invoice = xero.Invoice.build(...)
invoice.valid? # returns true
invoice.save # returns false
invoice.errors # returns empty array []
invoice.save! # raises Xeroizer::ApiException

How do I know what caused invoice.save to return false?

[virajvchavan]

Update: found the specific error by adding a logger and seeing the actual HTTP response. The item_code I sent for the line_items was incorrect.
Such obvious errors should be handled/shown in the code itself instead of raising a generic Xeroizer::ApiException exception.

[toxaq]

I have the same problem. Upgrading to 3.00 has broken validation errors on all models.

[toxaq]

It seems in version 2.18 the save! (bang) method was introduced. Previously validation errors would throw an exception. There is no code, that I could find, that will populate the errors from the validation errors in the XML response.

If you use the bang method (save!) then you can catch the original validation exception and handle this yourself.

[rjaus]

I'm not seeing this behaviour.

3.0.0 :034 > invoice = client.Invoice.build
 => #<Xeroizer::Record::Invoice > 
3.0.0 :035 > invoice.valid?
 => false 
3.0.0 :036 > invoice.save
 => false 
3.0.0 :037 > invoice.errors
 => [[:date, "can't be blank"], [:type, "not one of ACCPAY, ACCREC"], [:contact, "must be valid"]] 
3.0.0 :038 > invoice.save!
Traceback (most recent call last):
        5: from /Users/riley.james/.rvm/rubies/ruby-3.0.0/bin/irb:23:in `<main>'
        4: from /Users/riley.james/.rvm/rubies/ruby-3.0.0/bin/irb:23:in `load'
        3: from /Users/riley.james/.rvm/rubies/ruby-3.0.0/lib/ruby/gems/3.0.0/gems/irb-1.3.0/exe/irb:11:in `<top (required)>'
        2: from (irb):38:in `<main>'
        1: from /Users/riley.james/Code/xeroizer/lib/xeroizer/record/base.rb:116:in `save!'
Xeroizer::RecordInvalid (Xeroizer::RecordInvalid)

Did someone fix it?

Feel free to re-open if this issue persists, but I'm unable to reproduce based on the above details provided.

[toxaq]

Those are all local validation errors. From memory, this was about errors returned from the API not been populated anywhere. So if the contact already exists for example, the response is just captured and disregarded.

[rjaus]

Right, thanks for details.

Yes, I've also noticed that OAuth related errors such as token invalid / expired, return false, but it's unclear unless you're catching those errors from save!

[toxaq]

Yes, our original logic utilised the nature of save throwing errors so it was simple for us to switch to the bang method (after the 2.18 change) to get back to where we were. Ideally though the save method would do as OP expected.

[rjaus]

Ok, seems like a relatively easy fix. None of the error classes were included in the OAuth2 class.

I'll get a PR up shortly, @toxaq I'll ping you to review, be good to have some eyes on it as I'm a bit dubious on the test coverage.

edit: That's not it. Investigating further

[rjaus]

Looks like things changed when this commit came in:
3f7f173
4a181bc

But it's not super clear to me where to take it to improve.

[toxaq]

I would probably start around here:

xeroizer/lib/xeroizer/record/base.rb

Line 110 in b78a9a0

rescue XeroizerError => e

The exceptions are still being raised, as per the pre-2.18 functionality, they're just captured and dumped.

[toxaq]

Could be as simple as

self.errors[:base] << e.message

There's nothing else available error wise.

This is where they are extracted from the response

xeroizer/lib/xeroizer/exceptions.rb

Line 37 in cce6733

@parsed_xml.xpath("//ValidationError").each do |err|

[joseairosa]

Having the same issue here, any updates?

[timdiggins]

@rjaus I'm trying to work on a PR for this, but struggling to run individual tests. I'll make a separate issue for this (I don't normally use minitest, so could be a dumb error).

[timdiggins]

I've made a pretty crappy little PR for this - it does tick the basic box of providing an errors object, but it's not super nice.

I feel like the errors object (it's an array of arrays) could be abstracted and encapsulated (this is a pre-existing issue) so you don't have to know its structure to add an error and extract the messages. Could even go as far as adapting an ActiveModel::Errors approach.

I'm not too happy with using the Exceptions as the mechanism for communicating the validation errors between the http layer and the record layer. However I don't have a clear vision how to change this.

[timdiggins]

Yes, I've also noticed that OAuth related errors such as token invalid / expired, return false, but it's unclear unless you're catching those errors from save!

I think that these two errors should never be caught by save (they aren't validation errors, they are failures) and should be passed on to the library user more or less as is to handle. However this is probably a separate issue (though might be addressed by:

Perhaps the way to do this is to only allow .save to swallow the error (and return false) if there is a meaningful ValidationError to pass on, otherwise it should raise the (lower-level) error (including the response body). This is a bit of a weird contract for the .save method, but seems the most useful to me as a consumer of this library.

(see #551 (comment))