medic / cht-core

The CHT Core Framework makes it faster to build responsive, offline-first digital health apps that equip health workers to provide better care in their communities. It is a central resource of the Community Health Toolkit.
https://communityhealthtoolkit.org
GNU Affero General Public License v3.0
469 stars 217 forks source link

Broken form.xml attachment can prevent api server from starting #9618

Closed jkuester closed 1 week ago

jkuester commented 2 weeks ago

What feature do you want to improve?

When the api server starts, it tries regenerating new model.xml and form.html attachments for all the form:... docs (based on the xml attachment). This helps ensure that the server is always running with the proper version of the forms.

However, if the xml attachment for the form doc is invalid in such a way that xsltproc is unable to convert the file (e.g. this issue was observed in production when a ' was accidentally included in an xml tag), then the server will actually throw an error and fail to start.

Describe the improvement you'd like

This behavior is not ideal because in most deployment setups, the api server is the proxy for Couch traffic. The thrown error contains a clear description of the problem, and attaching a fixed version of the xml file will resolve the issue. However, there is no straightforward way to do this because you cannot remotely connect to Couch (since the api server is down).

To be clear, if you use the cht-conf convert-*-forms commands to generate your form xml, these commands should not produce invalid form xml files. Additionally, the cht-conf validate-*-forms commands (and the /api/v1/forms/validate endpoint) will fail when given an invalid xml. However, if you run the upload-*-forms commands with the --skip-validate flag, it will upload an invalid form xml attachment to the database. If the api server is running when this happens, it will log an error (when the model.xml and form.html attachments fail to generate for the updated form), but the server will continue to run. It will run until the next time it is restarted at which point it will fail to start as described above.

I think, instead of crashing the server when the form generation fails, it might be preferable to simply remove any existing model.xml and form.html attachments from the form doc, log an error, and continue starting the server. Removing the model.xml and form.html attachments will mean that users cannot load the form. When they try, this will get printed to the browser console (and the normal form error page will be shown to the user):

Error loading form Error: Error in XMLFormService : hasRequiredAttachments : The form "test_form" doesn't have required attachments

This behavior should at least point admins in the right direction if the missed the error logged during the server startup (and it will prevent anyone continuing to use old versions of the form).

Describe alternatives you've considered

As things are now, the most straightforward way to recover from this issue is to bash into the container running Couch. (e.g. if deploying via docker compose you can run something like this from the host server: docker exec -it cht_couchdb_1 bash) Then you can use curl to either completely delete the form:... doc or else upload a fixed version of the xml attachment.

dianabarsan commented 2 weeks ago

I would qualify this as a bug, rather than an improvement.

garethbowen commented 2 weeks ago

+1 Diana. API should never crash unless absolutely necessary (eg: out of memory).