Have store plugin handle 400 and 403

[mattflaschen]

Currently, there is special handling for 401, 404, and 500 (https://github.com/okfn/annotator/blob/master/src/plugin/store.coffee#L492)

However, 400 and 403 are both also useful. 400 is used for invalid requests. Although 401 is similar to 403, 401 responses are supposed to have WWW-Authenticate. 403 allows responding with an immediate failure without WWW-Authenticate.

This is similar to issue #5, but that's been closed for a long time, so I didn't comment there.

[Parent5446]

The store plugin should also be able to handle the 201 response code when writing annotations. Currently, it expects only the 303 status code, which, while technically OK, is not the most suitable status code for creating resources. The idea behind the 303 status code is that the response to the request can be found at another URL, but that's not semantically what's occurring here. When the plugin submits a POST to the creation endpoint, it results in the creation of a new annotation resource, which corresponds directly to the 201 Created status code. It should be noted that, like 303, 201 also says to put the URI of the new resource in the Location header.

[tilgovi]

Annotator doesn't currently expect any status code in particular when writing annotations. The only requirement is that the updated annotation data be returned in a JSON response. Whether that data is available at the URL used to create or update the annotation or is found at the URL indicated in the Location header is not really important.

It is a valid suggestion that annotator-store should include the Location header and return 201 on annotation creation. Currently it returns 200 and no Location header. I've opened an issue at openannotation/annotator-store#63 to track the pieces relevant to annotator-store.

The issue here as described by OP is mostly correct, but I would expand it to include all 4xx and 5xx responses.

[robcast]

The last time I tried the store plugin didn't work with the 303 status the store-spec proposes on creation of annotations so I had to make my store implementation return 200 and the JSON object.

I don't know about 201 but it may also be that this is an issue of Javascript access to the response in the browser.

As to 401, I hacked the Auth plugin to have a callback funktion when access to the Token requires authentication: https://github.com/robcast/annotator/blob/master/src/plugin/authlogin.coffee ;-)

[mattflaschen]

https://www.mediawiki.org/wiki/Extension:Annotator had to deal with these issues. 303 works in the browser's I've tested, for both Create and Update. I think this is because the browser follows the redirect (which I think is intended under REST principles). It's also using 401 for both user_not_logged_in and user_not_authorized. I think both should be 403, but that wouldn't work per my report.

It's not using 201

[tilgovi]

This should be solved at the same time as #266

[nickstenning]

From this point forward, I'm going to be keeping the Annotator issue tracker for bug reports only. Enhancements and feature requests should be made on the mailing list.

As this is a feature request, I'm going to close this issue. If you feel that I've miscategorised the discussion, and there is a genuine unaddressed bug, feel free to reopen with an explanation.

[tilgovi]

We just merged some code for annotator-store to return these errors. Since Annotator is handling some of them, it's easy for us to add a couple clauses with more descriptive error messages. I'll reopen this one.

[tilgovi]

It looks like the HttpStorage class on master has support for 400, 401, 403, 404, and 500. I think that means we can close this.