fails()'s convertResponseErrors() doesn't allow for using error 'meta' property or returning exact response passed

[cristinawithout]

I see convertResponseErrors() is converting errors to JSONAPI error format.

But I have the exact JSONAPI response expected from the server - which is an error object that uses the 'meta' property. Right now, I can't mock a fail with the expected response, because it doesn't support converting 'meta', and I can't just pass it the array of JSONAPI error objects I want it to return because it tries to convert them.

(Also, it's a bit of a hassle to convert my known JSONAPI response into the expected format so that it can convert it back to JSONAPI again.)

I can make a pull request, but not sure how you'd want to handle this. Or if there's a valid reason it's not allowing for passing the raw response you want it to come back with.

convertResponseErrors() could allow for not converting if an array is received like so. All the current tests still pass doing this:

  convertResponseErrors(object) {
    let errors = object.errors;
    if (Ember.isArray(errors)) {
        return errors;
    } else {
      let jsonAPIErrrors = [];
      Ember.assert('Your error REST Adapter style response must have an errors key. The errors hash format is: {errors: {description: ["bad"]}}', object.errors);
      for (let key in errors) {
        let description = Ember.typeOf(errors[key]) === "array" ? errors[key][0] : errors[key];
        let source = { pointer: "data/attributes/key" + key };
        let newError = { detail: description, title: "invalid " + key, source: source };
        jsonAPIErrrors.push(newError);
      }
      return { errors: jsonAPIErrrors };
    }
  }

But it might be a better to explicitly let someone disable convertResponseErrors() than trying to guess at when to do it vs not thereby allowing the option for fails() to return exactly what you sent it no matter what the format.

Such as changing fails() to do this and including 'convertResponse' (or some other name) as an option:

  fails(options = {}) {
    if (options.status) {
      Ember.assert(`[ember-data-factory-guy] 'fails' method status code must be 3XX, 4XX or 5XX,
        you are using: ${options.status}`, options.status.toString().match(/^([345]\d{2})/));
    }
    this.status = options.status || 500;
    if (options.response) {
      let errors;
      if (false !== options.convertResponse) {
        errors  = FactoryGuy.fixtureBuilder.convertResponseErrors(options.response, this.status);
      } else {
        errors = options.response;
      }
      this.errorResponse = errors;
    }
    return this;
  }

Thoughts?

[danielspaniel]

This has been a thorn in my side for months. EmberData has gotten better at smoothing out the error response lately, and that convertReponseErrors method was meant to do that.
For the tests to work in multi adapter mode, I think was the main idea behind this conversion.
But overall you are right, there is no reason why you should not be able to pass in your own errors.

Let me play around with those errors again, and get more clarity on this one.

[danielspaniel]

@cristinawithout , can you give me example of an error response you want to send
and how you expect to consume it ( when it returns )
I want to make some tests and will use some real examples

[cristinawithout]

Here ya go.

{
    "errors": [
        {
            "status": "422",
            "code": "http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html",
            "title": "Unprocessable Entity",
            "detail": "Failed Validation",
            "meta": {
                "validation_messages": {
                    "name": {
                        "regexNotMatch": "The input can only contain letters",
                        "stringLengthTooShort": "The input is less than 2 characters long"
                    }
                }
            }
        }
    ]
}

During .catch() after model.save(), 'meta.validation_messages' is converted into an object with an array of errors by property name, which will be used in the template to show beneath the form input that corresponds to that property. So from the above response, a 'formErrors' property on the component or controller ends up as:

{
  name: ['The input can only contain letters', 'The input is less than 2 characters long']
}

In some cases, 'meta.validation_messages' comes back with nested objects and arrays also. I can give example of that too if you need. But if it's working for the above and the 'meta.validation' messages is being returned exactly as it's given, all the variations should work.

[danielspaniel]

yikers, never heard of meta in errors object before .. but there it is in the json api spec.
this is great info .. thanks.
I am going to dig into error handling this weekend and see if I can get that spiffed up and polished.

[danielspaniel]

I wonder, do you think there is even a valid use case for factory guy formatting the errors at all.
I recall thinking it would be nice to be able to use simpler format=> {errors: [{name: 'too silly'}]} and then convert that to the json api style errors or rest style ( which this almost is ) .. and make the response setup easier, but it is perhaps better to either have that as separate function => makeMyErrorsHashForMe([{name: 'silly'}] ..
which is what I am doing now but not telling you ( this would make it more explicit )
and / or ..keep doing what I am doing and use the option that you suggested ( dontSetupMyErrorsPlease )
and or stop formatting completely and let the user do whatever .. even if it is wrong

any preference for any of these options?

[cristinawithout]

I was wondering myself if there was a good reason for automatically formatting the errors. Until this week, all my apps' tests have been using old versions. I wasn't expecting that behavior, so it took me a minute to figure out what was happening to my error response.

I suppose if you have a simple fail error, it could be more convenient. Personally, I'd rather just give it the error object tho so the error being received is obvious when reading the test.

I like having the conversion as a separate function. Tho that'll mean people will have to change their existing tests - the option to disable it keeps the default behavior for people's existing tests.

But I would prefer a separate function and no conversion by default and think that's more intuitive.

[danielspaniel]

I agree.

I may have to keep it the way it is and add the option, but as I ponder, I think the number of tests that return error responses is pretty small so breaking that one will probably not hurt too many people's feelings.

I guess I will ponder some more.

[danielspaniel]

I reckon v 2.7.0-beta.9 will solve your problem @cristinawithout because I am formatting automatically and giving the option to skip that formatting with convertErrors flag

[danielspaniel]

might be nice to have a global setting to never format the errors ? let me know if this is needed because using that convertErrors flag gets tedious

The reason I decided to keep formatting the errors is because the errors have to be formatted as json api for them to be accepted by all ember-data adapters. Given that .. it is really really nice to be able to just make the simple { errors: { field: 'the issue' } and get the whole thing converted for you.

I could have missed something though, or gotten mixed up in my reasoning, so feel free to let me know what I am missing.

Oh .. yeah .. and the function is available as:

FactoryGuy.fixtureBuilder.convertResponseErrors(errors);

[cristinawithout]

Thanks. convertErrors param works great. I don't' have too many fail tests that need a specific response. So the param is fine.