BadRequestError, UnauthorizedError, NotFoundError, etc. not raised

[adrienverge]

I'm opening an issue, I hope it's the right way to discuss on this. If the problem is known and not implemented yet, I'll be happy to help and/or contribute.

---

aiocouch defines various exception types (NotFoundError, UnauthorizedError, etc.) but doesn't raise them upon HTTP errors. Instead, raw aiohttp.client_exceptions.ClientResponseError exceptions are returned to the caller.

As a result, it is not possible to easily handle error cases, like this:

try:
    doc = await database["missing"]
except NotFoundError:
    # handle exception

Instead, I need to write this cumbersome code:

try:
    doc = await database["missing"]
except aiohttp.client_exceptions.ClientResponseError as e:
    if e.status == 401:
        raise InvalidUsage(401, 'Unauthorized')
    raise

As a comparison, with https://github.com/djc/couchdb-python one could use:

try:
    doc = database["missing"]
except couchdb.http.ResourceNotFound:
   # handle exception

[bmario]

That is strange. Users shouldn't see aiohttp exceptions. These should be captured by the @raises decorator and turned into the internal exceptions. Can you provide a stack trace?

edit: lol, sorry for the ping :(

[bmario]

In fact, there is a test for that.

https://github.com/metricq/aiocouch/blob/master/tests/test_document.py#L51

At least for missing documents, you seem to have an authorization problem though. There's no check for that

[bmario]

Okay, so I think what is happening for you is that your credentials aren't working. So no matter the request, you'll get a 401 status code. I added a check at the enter of the with-statement, so new connections will do a sanity check if the session is valid at all.

[adrienverge]

1. OK I got it: aiohttp exceptions are wrapped for documents, but not for databases. For example, with the following code:

   async with aiocouch.CouchDB(conf.couchdb_url, 'root', 'password') as couch:
      try:
          users = await couch['database']
      except aiocouch.UnauthorizedError:
          pass
      except aiocouch.NotFoundError:
          pass

   raises a KeyError in case of 404, and a aiohttp.client_exceptions.ClientResponseError for 401 (feel free to ask if you want the stacktrace).

2. Okay, so I think what is happening for you is that your credentials aren't working.

   Good idea, but no, that was not the problem.

   I added a check at the enter of the with-statement, so new connections will do a sanity check if the session is valid at all.

   I think that's a bad idea because:

   • It doesn't help (I tried with and without your new commit, i.e. on master f613c1f and master 7197f9d), and in both cases:
     • 401s are properly turned into aiocouch.UnauthorizedError for documents,
     • 401s are returned as aiohttp exceptions for databases.
   • Adding an extra HTTP call when entering every with CouchDB() is probably not what the user expects/wants (at least, not what I expect/want :slightly_smiling_face:). Adding a call to /_session has implications. Other libs don't do that.

[bmario]

It didn't work, because I accidentally put the check in the wrong place. Can you try the current master again?

Adding an extra HTTP call when entering every with CouchDB() is probably not what the user expects/wants (at least, not what I expect/want 🙂). Adding a call to /_session has implications. Other libs don't do that.

I'm not too convinced of that, in the end, making sure the provided credentials are valid is a normal part of communication with a server. Granted, usually, you'd have the server decline you during the connection process.

Let us put that aside. I still think your credentials aren't valid. Because the await couch['database'] statement sends a HEAD request to the /database endpoint of the CouchDB server. According to the documentation, that endpoint shouldn't return a 401. And accordingly, I haven't wrapped the ClientResponseError for a 401 in that case. So either the documentation is wrong or your credentials.

[adrienverge]

Hey @bmario, thanks for following this up.

I confirm that with latest master (f1abf39), I now get a proper aiocouch.exception.UnauthorizedError on await couch['_users'] and a proper aiocouch.exception.NotFoundError on await couch['doesntexist'].

• According to the documentation, that endpoint shouldn't return a 401.

  I think that documentation omits the 401 possibility because they consider it obvious. A real CouchDB server (2.3.1) returns a 401 on HEAD with invalid username/password or expired cookie:

  $ curl -v -X HEAD root:bad@localhost:15984/db HTTP/1.1 401 Unauthorized

• I still think your credentials aren't valid.

  Yes, but it's intended. I'm trying to cleanly handle that case (users sending invalid/expired credentials).

• Anyway, I think HTTP 401 should be turned into aiocouch.exception.UnauthorizedError (instead of passing raw aiohttp exceptions) even after aiocouch.CouchDB() initialization, because:

  • a cookie session can expire, hence a CouchDB() object that passed __aenter__ could get a 401 on __getitem__('db') a few minutes later,
  • username/password credentials can be revoked (just change the password), hence a CouchDB() object that passed __aenter__ can get 401s after revocation.

• If you agree with my previous point, there is not need to add an extra HTTP request on /_session for every CouchDB() object, because 401 would be handled per-request.

  For info, this extra HTTP call would be a blocker for us to switch from couchdb-python to aiocouch, because it would double the number of requests on our CouchDB servers, which wouldn't handle the load (our Python code works as a proxy for 10000+ of users, so we have to create CouchDB() objects on the fly, whose lifetime is ~300 ms).

[bmario]

You raised fair points, so I removed the credentials check and wrapped those implicit status codes. One can still explicitly check the provided credentials using await couchdb.check_credentials() if needed.

One remark, when Couchdb returns a 401 status code, I don't see an elegant solution to check whether there is a problem with the authentication, i.e., the provided credentials are valid, or with the authorization, e.g., administrator permissions required for the _users database. The difference is for me a user error vs. configuration error. This issue, I sadly have to adopt from Couchdb, which should probably use 403 in some cases.

Anyways, can you try the latest commit once more and report back if my changes solved your issue?

[adrienverge]

Thanks! I just tested and it works fine :+1:

I don't see an elegant solution to check whether there is a problem with the authentication, i.e., the provided credentials are valid, or with the authorization, e.g., administrator permissions required for the _users database.

I totally agree, in my opinion that's why the difference between 401 Unauthorized and 403 Forbidden was invented. It's sad that CouchDB doesn't use it.

[bmario]

I'm closing this issue then.