Document federation errors

[pcapriotti]

This adds documentation of federation errors to Swagger. These are not response entries by endpoints, but simply a list of errors (with some brief explanations) divided into categories. They apply more or less to all endpoints that have anything to do with federation.

I also pasted it here so it is possible to see it rendered without building anything: https://gist.github.com/pcapriotti/e50f61c3a8f6abab900e25c9ff3457aa.

TODO

• Remove unused errors
• Move all error label assignments to a single module
• Rethink some of the error codes
  After some discussion, we decided the error codes are mostly fine
• Add suggested client behaviour for each error

Checklist

• The PR Title explains the impact of the change.
• The PR description provides context as to why the change should occur and what the code contributes to that effect. This could also be a link to a JIRA ticket or a Github issue, if there is one.
• If end-points have been added or changed: the endpoint / config-flag checklist (see Wire-employee only backend wiki page) has been followed.
• If a schema migration has been added, I ran make git-add-cassandra-schema to update the cassandra schema documentation.
• Section Unreleased of CHANGELOG.md contains the following bits of information:
  • A line with the title and number of the PR in one or more suitable sub-sections.
  • If /a: measures to be taken by instance operators.
  • If /a: list of cassandra migrations.
  • If public end-points have been changed or added: does nginz need upgrade?
  • If internal end-points have been added or changed: which services have to be deployed in a specific order?

[mdimjasevic]

A couple of minor suggestions are inlined.

[jschaul]

This is a federation-api-backwards-incompatible change (since router.proto changes), which is fine right now, but we'd need to be more careful about this in the future. Could be added to the changelog like here: https://github.com/wireapp/wire-server/blob/develop/CHANGELOG.md#internal-federation-api-changes

[pcapriotti]

If we had kept the existing field indices, then I think this sort of change should be backwards compatible. Is this right?

[jschaul]

Yes, if the meaning and the numbers stay, but only things are added, it's backwards compatible. If indices change or are removed it's not.
So far so good, no need for backwards compatibility concerns just yet.

[pcapriotti]

Thanks for approving @jschaul. I marked this PR as a draft, because I wanted to use it to work on reorganising the error codes and clean up error handling. Should I instead merge it as is, and do that in a separate PR?

[jschaul]

Thanks for approving @jschaul. I marked this PR as a draft, because I wanted to use it to work on reorganising the error codes and clean up error handling. Should I instead merge it as is, and do that in a separate PR?

As you wish, I think this is not urgent to get merged, so feel free to keep it around and organize errors a but differently first. Whatever is more convenient.

Another thought crossed my mind: I noticed when doing a manual test that currently the behaviour of the webapp on a 400 (federation not allowed) is, they stop. And on the 525 errors, the webapp retried the request about 10 times. So I wonder if there should be some guidance we can give on these different errors about what e.g. the client could do (retry later, or not), or what the user could do. Some of this discussion could happen with involvement of more parties, but some things could also be added to the description perhaps.

[pcapriotti]

Another thought crossed my mind: I noticed when doing a manual test that currently the behaviour of the webapp on a 400 (federation not allowed) is, they stop. And on the 525 errors, the webapp retried the request about 10 times. So I wonder if there should be some guidance we can give on these different errors about what e.g. the client could do (retry later, or not), or what the user could do. Some of this discussion could happen with involvement of more parties, but some things could also be added to the description perhaps.

Good point, I added it as a TODO item.