CORS filter causes error responses go to success handlers in AngularJs

[josegar74]

Probably related to http://www.jamessturtevant.com/posts/AngularJS-HTTP-Service-Success-Handler-400-Response-Code-and-Interceptors/

Disabling the CORS filter works fine. To disable it, remove the following lines from GnModule:

• https://github.com/geonetwork/core-geonetwork/blob/develop/web-ui/src/main/resources/catalog/js/GnModule.js#L32
• https://github.com/geonetwork/core-geonetwork/blob/develop/web-ui/src/main/resources/catalog/js/GnModule.js#L67

Checked the code and seem not entering in responseError handler, so probably an issue in this code:

https://github.com/geonetwork/core-geonetwork/blob/develop/web-ui/src/main/resources/catalog/components/utility/CORSInterceptor.js#L43-L59

[Delawen]

It should be related to the bottom lines, where the error in handled. But it's weird, as a 404 error should be skipped... checking.

[fxprunayre]

This one is becoming more and more urgent to fix as it trigger many side effects on the Angular app. eg. create a group which already exist return success.

[josegar74]

Rechecked again and the code indeed enters in the responseError handler. I have change the following line and at least the issue with the duplicated groups error message is solved:

https://github.com/geonetwork/core-geonetwork/blob/develop/web-ui/src/main/resources/catalog/components/utility/CORSInterceptor.js#L92

to this

return $q.reject(response);

The problem is that the error handler has several return points and the code is a bit unclear to me. Would be good if @Delawen can take a look to check if all return points should use $q.reject(response)

[josegar74]

Fixed the previous case, that should solve issues with the requests to the GeoNetwork services that return errors.

But as indicated previously, the error handler has several exit points, that should be reviewed also. I think in all cases, should do this:

return $q.reject(response);

[fxprunayre]

Can we close ?

[Delawen]

I think so. @josegar74 ?

[josegar74]

@Delawen I proposed a solution for other cases, please check the comment from Aug 5. That is not committed and requires a review from you. I think you developed this code and should be clear to you.

[fxprunayre]

Do we have any use cases where errors are not reported properly ?

[josegar74]

@fxprunayre Any error response from GeoNetwork services went to the success handler, if not applied the fix from https://github.com/geonetwork/core-geonetwork/commit/12dfbdaca25b5f400732db42e7f14d15c1af7735

But that fix only applies to GeoNetwork services calls. This other code seem used when doing Ajax calls to external sites: https://github.com/geonetwork/core-geonetwork/blob/3.2.x/web-ui/src/main/resources/catalog/components/utility/CORSInterceptor.js#L72-L92

That is what requires revision, but I don't have any use case, not clear where in the UI is used that code. I propose a solution (see comment from 5 August), but would be good if @Delawen can take a look to confirm that would not cause any unwanted side effect.

[pvgenuchten]

I think this is a related case https://github.com/geonetwork/core-geonetwork/issues/1694 no error displayed if upload fails

[fxprunayre]

I think this is a related case #1694 no error displayed if upload fails

If upload fails, I've exception message eg. if file is too big

Any other cases to cover ?

Related to that, I improved error reporting when something is wrong for suggestion (ie when XSL error happens).

So, we let @Delawen check if there is any other cases to handle in the interceptor ? and if you've any error not reported properly open a new ticket.

[fxprunayre]

So far, the interceptor looks quite good now. Closing unless we find other cases to cover.