copr / copr

Clone

#2099 frontend: user-friendly error when there is a database problem
Merged 2 years ago by praiskup. Opened 2 years ago by frostyx.
copr/ frostyx/copr no-db-error  into  main

file modified
+7 -1 
@@ -5,6 +5,7 @@ 
 

  import logging
 

  

  import flask
 

+ from sqlalchemy.exc import SQLAlchemyError
 

  from werkzeug.exceptions import HTTPException, NotFound, GatewayTimeout
 

  from coprs.exceptions import CoprHttpException
 

  from coprs.views.misc import (
 
@@ -89,8 +90,13 @@ 
 

          if code in error_views:
 

              return error_views[code](message)
 

  

+         message = "Server error, contact admin"
 

+         if isinstance(error, SQLAlchemyError):
 

+             code = 500
 

+             message = "Database error, contact admin"
 

+ 

          self._log_admin_only_exception()
 

-         return generic_error("Server error, contact admin", code)
 

+         return generic_error(message, code)
 

  

  

  class APIErrorHandler(BaseErrorHandler):

file modified
+8 
@@ -13,4 +13,12 @@ 
 

    {% else %}
 

      Something went wrong
 

    {% endif %}
 

+ 

+   {% if config.debug or config.env != "production" %}

How the config.env is defined? I quickly grepped throught the sources, and it doesn't seem to be defined.

It is defined here:

$ git grep -i "env=" |grep copr.conf
docker/frontend/files/etc/copr/copr.conf:ENV="local"
frontend/coprs_frontend/config/copr.conf:ENV="devel"

And for production we set it in ansible I think.

 
+     <p>
 

+       Administrators should take a look at
 

+       <code>/var/log/copr-frontend/frontend.log</code>
 

+       to analyze the full error output.
 

+     </p>
 

+   {% endif %}
 

  {% endblock %}

Fix #2076

Build succeeded.

Can't we somehow easily detect the database is not initialized? This feels
weird :-/ I would be afraid we are losing the original error message in
a production deployment in some buggy situations....

There still is the self._log_admin_only_exception() that logs full traceback saying for example that the coprdb database doesn't exist or that a column is missing and migrations needed to be run.

This PR only changes what we show in the web UI. I intentionally didn't want to print the error message there in case it contained some not-so-public information. Also, it won't be any help for users to see the original error message because there is nothing they can do about this but contact us. Same as for the "Server error, contact admin" error message, that we already have.

Not that the database-related errors are currently displayed anyway :-). Currently, we show something like ValueError: status code is not a 3 digit integer.

Can't we somehow easily detect the database is not initialized?

I think in our frontend/coprs_frontend/coprs/__init__.py we can for example try to query the latest alembic migration (select version_num from alembic_version;) and if it is not what we would expect or we get any error that the database doesn't exist or something, we can print an error. But I don't see any point in it because I think we would show the same kind of error that I am already showing in this PR but in a more complicated way with one more database query on each page refresh :-)

ACK

There still is the self._log_admin_only_exception() that logs full
traceback saying for example that the coprdb database doesn't exist or
that a column is missing and migrations needed to be run.

Can we somehow tell the newcomers where to look for the traceback? E.g.
when debugging is enabled (which should be in podman compose up by
default?) then we could say that:

Administrators should take a look at /var/log/asdfasdfas.log to
analyze the full error output.

Without this (or something alike) I'm afraid we'll face still the same
reports.

rebased onto fcfb9862aa62e92d968e0ef304e9789bec5436dc
2 years ago

Build succeeded.

rebased onto 4954eec1fa5896051bde9539a03d81e5bd34ca92
2 years ago

Can we somehow tell the newcomers where to look for the traceback? E.g.
when debugging is enabled

Updated, PTAL

Build succeeded.

How the config.env is defined? I quickly grepped throught the sources, and it doesn't seem to be defined.

It is defined here:

$ git grep -i "env=" |grep copr.conf
docker/frontend/files/etc/copr/copr.conf:ENV="local"
frontend/coprs_frontend/config/copr.conf:ENV="devel"

And for production we set it in ansible I think.

Ok, it's just a bit confusing. +1 then

rebased onto a701062
2 years ago

Build succeeded.

Pull-Request has been merged by praiskup
2 years ago
Metadata
None
No Tags
Changes Summary 2
+7 -1
file changed
frontend/coprs_frontend/coprs/error_handlers.py
+8 -0
file changed
frontend/coprs_frontend/coprs/templates/_error.html
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.