Store service filters erros provided in exception

[mhankus]

Merge of #8084 introduced some breaking changes to error handling.
Before this commit, when InvalidError was created with array of errors object, it was possible to inspect this array in application, but now it is filtered out - I assume that in the following code (code fragment from store-service.ts)

if (errors) {
    Object.keys(errors).forEach((key) => {
      let messages = makeArray(errors[key]);
      for (let i = 0; i < messages.length; i++) {
        let title = 'Invalid Attribute';
        let pointer = `/data/attributes/${key}`;
        if (key === PRIMARY_ATTRIBUTE_KEY) {
          title = 'Invalid Document';
          pointer = `/data`;
        }
        out.push({
          title: title,
          detail: messages[i],
          source: {
            pointer: pointer,
          },
        });
      }
    });
  }

As a result code property is filtered out (and other properties mentioned in https://jsonapi.org/format/#errors).
JSON:API specification states that

code: an application-specific error code, expressed as a string value.

My application relayed on error codes, but now they are not available, and requires major refactoring to update to new version of Ember Data.
I'm not sure if overriding title to a fixed English value is a good idea (for example if title was localized on server side - it can also break some apps).

[runspired]

Before this commit, when InvalidError was created with array of errors object, it was possible to inspect this array in application, but now it is filtered out

The original Error is still thrown and should still contain the errors property

As a result code property is filtered out

we don't filter anything out. In fact just the opposite!

If the serializer had implemented the (semi-secret) method called serializer.extractErrors, we pass them to it unmodified. We then transform the result back into JSON:API errors array format. This transformation is roughly unchanged for ~3-4 years, here it is in 4.4 and 3.16

If the serializer does not implement serializer.extractErrors (it is preferred and better that it does not) then we pass the errors entirely unmodified to the cache

We store these errors still unmodified

When @ember-data/model requests these errors it will transform them into the shape used by the Errors proxy (record.errors). This involves determining the key the error is associated to and the simple string for the message. see code, while the Model codepath changed, it was never exposing any additional properties other than these.

I suspect the issue you have here is that we leaked the original errors in the past by not updating the reference after normalizing (we threw without normalizing here which was then caught here and rethrown.

We could potentially restore this, though it would in many ways be a bug. I suspect the real solution here is for you to simply delete the serializer's extractErrors method as it serves you no purpose it seems and is the reason this property is lost to you.

[runspired]

closing for now, happy to reopen for further discussion if it sounds merrited.

[Techn1x]

happy to reopen for further discussion if it sounds merrited.

@runspired yes please

I've just attempted to upgrade my application from ED 4.6.4 to 4.7.3 and observed the same behaviour - exactly when the PR was released. I think there's a regression with error handling here.

The original Error is still thrown and should still contain the errors property

Correct - except the errors key in the rethrown error is overwritten inside adapterDidInvalidate

data/packages/store/addon/-private/store-service.ts

Lines 2695 to 2697 in 3bc74cb

	if (serializer && typeof serializer.extractErrors === 'function') {
	let errorsHash = serializer.extractErrors(store, store.modelFor(identifier.type), error, identifier.id);
	error.errors = errorsHashToArray(errorsHash);

After the code changes, serializer.extractErrors() returns an empty array for my errors. OK, so even if I implement extractErrors to be a passthrough and return my server error back as is, the errorsHashToArray() function then mangles it and sets errror.errors to that mangled error.

After 4.7.3 there's no way to actually return the error as the server gave it.

For example, my error is like this;

{
  errors: [{title: 'Unprocessable Entity', status: 422, detail: 'subscription no more student licences'}],
  isAdapterError: true
  message: "The adapter rejected the commit because it was invalid"
  name: "Error",
  stack: "......"
}

My application expects to be able to use the errors array as is.

} catch (e) {
  if (e.errors[0].status === 422) doThing()
}

If the serializer had implemented the (semi-secret) method called serializer.extractErrors, we pass them to it unmodified.

If the serializer does not implement serializer.extractErrors (it is preferred and better that it does not) then we pass the errors entirely unmodified to the cache

We store these errors still unmodified

Each time you've claimed the error is passed/stored unmodified, it's actually had its error.errors property modified

I suspect the issue you have here is that we leaked the original errors in the past by not updating the reference after normalizing

That sounds about right. But ... what am I supposed to do? The server returns errors that don't adhere to jsonapi spec EDIT: it does, see next comment, and as far as I can tell there isn't a function I can override to deal with that?

I suspect the real solution here is for you to simply delete the serializer's extractErrors method as it serves you no purpose it seems and is the reason this property is lost to you.

I've tried implementing a passthrough for the serializer.extractErrors() function (as I mentioned above), but because the errors are still run through errorsHashToArray() it makes a mess of my server error, and is not how it was pre-4.7.3

---

I think I would have been more prepared for this had there been a mention in the changelog or breaking changes or something. This was really painful to debug

[Techn1x]

The server returns errors that don't adhere to jsonapi spec

Actually, I believe that error I gave and most of the errors I am having issues with do infact adhere to the JSONAPI spec.

Both the serializer.extractErrors() and errorsHashToArray functions expect the original error to have had source, else error.errors is overwritten with a blank array.

https://github.com/emberjs/data/blame/6faabaf551d06d7a6bf39f738195775c725fce9d/packages/serializer/src/json.js#L1479

But the spec says that it can be omitted

https://jsonapi.org/format/#errors

source: an object containing references to the primary source of the error. It SHOULD include one of the following members or be omitted

---

Regardless, I then tried adding in the error/source/pointer/data as below, to indicate that the error isn't meant for any attributes. Unforunately, the errors array is still not passed through unmodified

extractErrors(store, typeClass, payload, id) {
    if (payload && typeof payload === 'object' && payload.errors) {
      payload.errors.forEach((error) => {
        if (!error.source) {
          error.source = { pointer: 'data' }
        }
      })
    }
    return super.extractErrors(store, typeClass, payload, id);

[Techn1x]

It's looking like my only option here will be to implement extractErrors() to store the server error on a new property, and then refactor the error handling throughout my application to instead point to that.

But if you've got any better suggestions, I'll take anything over that option

[runspired]

I think you want no extractErrors hook at all ...

[runspired]

would be happy to pair btw, there's a couple of nuances here and could be a weird interplay of things.

[Techn1x]

I think you want no extractErrors hook at all ...

I didn't even think of that..
extractErrors: false would skip the code block in question, since the typeof extractErrors === 'function' condition would be false

Would that be considered a public API thing to do? Is that likely to have other ramifications? (do other parts of ember-data rely on these errors having been extracted & formatted?)

---

would be happy to pair btw, there's a couple of nuances here and could be a weird interplay of things.

I appreciate that offer - I may take you up on it if I have trouble but I do think I have grokked the relevant aspects of this

[Techn1x]

Ok - I've thought a bit more about this.

This transformation is roughly unchanged for ~3-4 years, here it is in 4.4 and 3.16

While that's true, that code that processes errors hadn't changed in a long time, what is also true is that nobody was forced to use it for a long time..

I think a big part of the reason I'm having difficulty with the changes isn't necessarily that the errors are being parsed correctly now without an escape hatch - it's because the errorsHashToArray() function doesn't set most of the properties that can legally exist on a JsonApiError, which are typically used by my application (things like code and status). It just sets the most basic properties..

data/packages/legacy-compat/src/legacy-network-handler/legacy-network-handler.ts

Lines 277 to 283 in bf5b0d0

	out.push({
	title: title,
	detail: messages[i],
	source: {
	pointer: pointer,
	},
	});

---

That functions' return type is JsonApiError . If it were to return all the other items that can exist in a JsonApiError

data/ember-data-types/q/record-data-json-api.ts

Lines 25 to 41 in bf5b0d0

	export interface JsonApiError {
	id?: string;
	title?: string;
	detail?: string;
	links?: {
	about?: Link;
	type?: Link;
	};
	status?: string;
	code?: string;
	source?: {
	pointer: string;
	parameter?: string;
	header?: string;
	};
	meta?: Meta;
	}

then I can add the source/pointer: "data" in my extractErrors hook as I gave in a prior comment, and then the parsed errors will contain all the relevant (and spec-compliant!) response error data, instead of just throwing it away.

I suspect it would be resolved to the point where most of my error handling won't even need to change, and likely the same for other applications.

This would also solve the OP's need for the missing the code property.

I'm going to put a PR together with what I think needs to change, to help explain.

[Techn1x]

I've put this PR together
#8669

[Techn1x]

The PR I mentioned is a proof of concept if we wanted to patch the legacy code to still work the same as it used to, but I've instead now opted to set extractErrors = false in my json api serializers to avoid the problematic code

Hopefully someone out there finds that useful

[azhiv]

@runspired We are using JSON (not json api) serializer (which already implements extractErrors) and receive the following error from the BE:

{
  errors: [{
    code: '^error_message^',
    meta: { token_args: ['arg1', 'arg2'] },
    status: '422',
  }],
}

So the problem is that in the catch clause of this.model.save().catch(error => ...) the error.errors is an empty array in v4.8 whereas it contains the desired error in v3.28. As far as I could understand the extractErrors of 3.28 (either) didn't result in any hash of errors in the example above, but rather the errors array was somehow preserved when propagated up through Ember Data internals.
Is there a simple way to retain the previous behaviour without modifying the backend? If not, what changes have to be done in order to align with the current paradigm?

[runspired]

@azhiv sounds like in your case you weren't using EmberData's record.errors behavior before at all. Simply removing extractErrors should work in that case I think.

[azhiv]

@runspired Thank you, this seems to fix the problem.