From c2bcfb52aa7987cfc91c6f7f90a1baeca27203b7 Mon Sep 17 00:00:00 2001 From: Lauryn Menard Date: Thu, 21 Apr 2022 17:55:05 +0200 Subject: [PATCH] api-tests: Reduce error output for `/register` openapi validation. For descriptive endpoints, such as `/register`, that might raise Schema Validation errors via `validate_against_openapi_schema`, omits the OpenAPI schema definition in the error output. Also omits the error instance definition in the error output when it is a jsonschema object with over 100 properties. This means that the test instance for objects, like user settings, will be printed in the error output, but the test instance for the entire endpoint will not be printed to the console. The omitted output can be thousands of lines long making it difficult to find the initial console output that actually helps the contributor with debugging. Adds a section in "Documenting REST API endpoints" about debugging and understanding these errors that is linked to in the error console output. --- docs/documentation/api.md | 183 +++++++++++++++++++++++++ docs/tutorials/new-feature-tutorial.md | 6 + zerver/openapi/openapi.py | 11 ++ 3 files changed, 200 insertions(+) diff --git a/docs/documentation/api.md b/docs/documentation/api.md index 93e172fa43..29bf51071e 100644 --- a/docs/documentation/api.md +++ b/docs/documentation/api.md @@ -335,3 +335,186 @@ it? There's several major benefits to this system: Using the standard OpenAPI format gives us flexibility, though; if we later choose to migrate to third-party tools, we don't need to redo the actual documentation work in order to migrate tools. + +## Debugging schema validation errors + +A common function used to validate and test Zulip's REST API is +`validate_against_openapi_schema`. It is used to verify that every +successful API response returned in the backend and documentation test +suites are a documented possibility in the API documentation. + +Therefore, when you add a new feature or setting to Zulip, you will most +likely need to update the API documentation (`zerver/openapi/zulip.yaml`) +in order to pass existing tests that use this function. Additionally, if +you're writing documentation for a new or undocumented REST API endpoint, +you'll want to use this function to validate and test your changes in +`zerver/openapi/python_examples.py`. + +Below are some examples to help you when debugging the schema validation +errors produced by `validate_against_openapi_schema`. Before reading +through the examples, we recommend reviewing the +[OpenAPI configuration](openapi.md) documentation if you're unfamiliar +with the format. + +If you use Visual Studio Code, an OpenAPI extension can be very helpful in +navigating Zulip's large and detailed OpenAPI file; see +`.vscode/extensions.json`. + +### Deconstructing the error output + +To start with a clear example, let's imagine that we are writing the +documentation for the REST API endpoint for uploading a file, +[POST /api/v1/user_uploads](https://zulip.com/api/upload-file). + +There are no parameters for this endpoint, and only one return value +specific to this endpoint, `uri`, which is the URL of the uploaded file. +If we comment out that return value and example from the existing API +documentation in `zerver/openapi/zulip.yaml`, e.g.: + +```yaml + /user_uploads: + post: + operationId: upload-file +... + responses: + "200": + description: Success. + content: + application/json: + schema: + allOf: + - $ref: "#/components/schemas/JsonSuccessBase" + - $ref: "#/components/schemas/SuccessDescription" + - additionalProperties: false + properties: + result: {} + msg: {} + # uri: + # type: string + # description: | + # The URI of the uploaded file. + example: + { + "msg": "", + "result": "success", + # "uri": "/user_uploads/1/4e/m2A3MSqFnWRLUf9SaPzQ0Up_/zulip.txt", + } +``` + +We will now get an error when we run the API documentation test suite +in the development environment (`tools/test-api`): + +```console +Running API tests... +2022-12-19 15:05:42.347 WARN [django.server] "POST /api/v1/users HTTP/1.1" 400 88 +Waiting for children to stop... +Traceback (most recent call last): + File "tools/test-api", line 93, in + test_the_api(client, nonadmin_client, owner_client) + File "/srv/zulip/zerver/openapi/python_examples.py", line 1636, in test_the_api + test_users(client, owner_client) + File "/srv/zulip/zerver/openapi/python_examples.py", line 1555, in test_users + upload_file(client) + File "/srv/zulip/zerver/openapi/python_examples.py", line 52, in _record_calls_wrapper + return test_func(*args, **kwargs) + File "/srv/zulip/zerver/openapi/python_examples.py", line 1284, in upload_file + validate_against_openapi_schema(result, "/user_uploads", "post", "200") + File "/srv/zulip/zerver/openapi/openapi.py", line 489, in validate_against_openapi_schema + raise SchemaError(message) from None +zerver.openapi.openapi.SchemaError: 1 response validation error(s) at post /api/v1/user_uploads (200): + +ValidationError: Additional properties are not allowed ('uri' was unexpected) + +Failed validating 'additionalProperties' in schema['allOf'][2]: + {'additionalProperties': False, + 'example': {'msg': '', + 'result': 'success', + 'properties': {'msg': {}, 'result': {}}} + +On instance: + {'msg': '', + 'result': 'success', + 'uri': '/user_uploads/2/85/XoqF0K7XEOLVGylgdpof80RB/img.jpg'} + +``` + +We can see in the traceback that a `SchemaError` was raised in +`validate_against_openapi_schema`: + +```console + File "/srv/zulip/zerver/openapi/openapi.py", line 478, in validate_against_openapi_schema + raise SchemaError(message) from None +``` + +The next line in the output, let's us know how many errors were found +and for what endpoint. + +```console +zerver.openapi.openapi.SchemaError: 1 response validation error(s) at post /api/v1/user_uploads (200): +``` + +As expected from commenting out the code above, there was one validation +error for the `POST /api/v1/user_uploads` endpoint. The next line gives +more information about that error. + +```console +ValidationError: Additional properties are not allowed ('uri' was unexpected) +``` + +We see that there was a `uri` value returned by the endpoint that hasn't +been documented. The next few lines of output, show us what return values +are documented (again due to our changes) for this endpoint. + +```console +Failed validating 'additionalProperties' in schema['allOf'][2]: + {'additionalProperties': False, + 'example': {'msg': '', + 'result': 'success', + 'properties': {'msg': {}, 'result': {}}} +``` + +And finally, we see the test instance that did not match our current +documentation, which includes the `uri` return value. + +```console +On instance: + {'msg': '', + 'result': 'success', + 'uri': '/user_uploads/2/85/XoqF0K7XEOLVGylgdpof80RB/img.jpg'} +``` + +This is a useful example because the endpoint's documentation is short +and straightforward, helping to easily identify the parts of the +error output that are useful in debugging these errors when testing the +API documentation. + +### Adding a realm setting + +Building on [the new feature tutorial](../tutorials/new-feature-tutorial.md) +example, if the realm setting for `mandatory_topics` was not documented +in the `POST /api/v1/register` endpoint, running `tools/test-api` in the +development environment would result in this error: + +```console +... +zerver.openapi.openapi.SchemaError: 1 response validation error(s) at post /api/v1/register (200): + +ValidationError: Additional properties are not allowed ('realm_mandatory_topics' was unexpected) + +Failed validating 'additionalProperties' in schema['allOf'][2]: + 'OpenAPI schema omitted due to length of output.' + +On instance: + 'Error instance omitted due to length of output.' +``` + +Because this endpoint is very long and descriptive, we do not print the +entire documentation schema (or test instance, in this case) to the +console. Doing so would print thousands of lines of output that are not +useful for debugging what is missing from the API documentation. + +The key information for debugging this endpoint is in the line beginning +with `ValidationError`. There we can see that the documentation does not +include the new `realm_mandatory_topics` boolean that we added in the +example feature tutorial, and we can look at other similar realm settings +to add the documentation for that new feature. diff --git a/docs/tutorials/new-feature-tutorial.md b/docs/tutorials/new-feature-tutorial.md index 8bb2dc92c8..c4605f0254 100644 --- a/docs/tutorials/new-feature-tutorial.md +++ b/docs/tutorials/new-feature-tutorial.md @@ -517,6 +517,12 @@ the setting enabled). Visit Zulip's [Django testing](../testing/testing-with-django.md) documentation to learn more about the backend testing framework. +Also note that you may already need to update the API documentation for +your new feature to pass new or existing backend tests at this point. +The tutorial for [writing REST API endpoints](../documentation/api.md) +can be a helpful resource, especially the section on [debugging schema +validation errors](../documentation//api.md#debugging-schema-validation-errors). + ### Update the frontend After completing the process of adding a new feature on the backend, diff --git a/zerver/openapi/openapi.py b/zerver/openapi/openapi.py index ca94bfd673..71f8fa0b5e 100644 --- a/zerver/openapi/openapi.py +++ b/zerver/openapi/openapi.py @@ -474,7 +474,18 @@ def validate_against_openapi_schema( validator_value=brief_error_validator_value, cause=error.cause, ) + # Some endpoints have long, descriptive OpenAPI schemas + # which, when printed to the console, do not assist with + # debugging, so we omit some of the error information. + if path in ["/register"] and isinstance(error, JsonSchemaValidationError): + error.schema = "OpenAPI schema omitted due to length of output." + if len(error.instance) > 100: + error.instance = "Error instance omitted due to length of output." message += f"\n\n{type(error).__name__}: {error}" + message += ( + "\n\nFor help debugging these errors see: " + "https://zulip.readthedocs.io/en/latest/documentation/api.html#debugging-schema-validation-errors" + ) raise SchemaError(message) from None return True