tracd incorrectly says "needs to be upgraded" if an exception is raised from needs_upgrade()

[Jun Omae]

Environment.needs_upgrade() raised a TypeError because installed plugin has incompatible method with Trac 1.3.x:

TypeError: environment_needs_upgrade() takes exactly 2 arguments (1 given)

When the error is raised, I get "needs to be upgraded" from tracd. It's incorrect, because the Trac environment is up-to-date.

Trac Error

TracError: The Trac Environment needs to be upgraded. Run:

  trac-admin "/home/jun66j5/var/trac/1.3-sqlite" upgrade

It seems the behavior is introduced in [15939] (#12680).

I consider that we should raise a TracError with Unable to check for upgrade message when an exception is raised from needs_upgrade():

trac/env.py

@@ -857 +857 @@
                 CacheManager(env).reset_metadata()
     else:
         env = Environment(env_path)
-        needs_upgrade = True
         try:
             needs_upgrade = env.needs_upgrade()
+        except TracError as e:
+            env.log.error("Exception caught while checking for upgrade: %s",
+                          exception_to_unicode(e))
+            raise
         except Exception as e:  # e.g. no database connection
             env.log.error("Exception caught while checking for upgrade: %s",
                           exception_to_unicode(e, traceback=True))
-        if needs_upgrade:
-            raise TracError(_('The Trac Environment needs to be upgraded. '
-                              'Run:\n\n  trac-admin "%(path)s" upgrade',
-                              path=env_path))
+            raise TracError(_("Unable to check for upgrade: %(err)s",
+                              err=exception_to_unicode(e)))
+        else:
+            if needs_upgrade:
+                raise TracError(_('The Trac Environment needs to be upgraded. '
+                                  'Run:\n\n  trac-admin "%(path)s" upgrade',
+                                  path=env_path))
 
     return env
 

After the patch, tracd says in this case:

Trac Error

TracError: Unable to check for upgrade: TypeError: environment_needs_upgrade() takes exactly 2 arguments (1 given)}

[Christian Boos]

Good idea, but reporting which plugin raised such an error seems also very useful in this case (so that one could disable it if needed).

[Jun Omae]

Agreed. I reworked the patch based on your suggestion.

Revised patch:

trac/env.py

@@ -745 +745 @@
     def needs_upgrade(self):
         """Return whether the environment needs to be upgraded."""
         for participant in self.setup_participants:
-            with self.component_guard(participant, reraise=True):
-                if participant.environment_needs_upgrade():
-                    self.log.warning(
-                        "Component %s requires environment upgrade",
-                        participant)
-                    return True
+            try:
+                with self.component_guard(participant, reraise=True):
+                    if participant.environment_needs_upgrade():
+                        self.log.warning(
+                            "Component %s requires environment upgrade",
+                            participant)
+                        return True
+            except Exception as e:
+                raise TracError(_("Unable to check for upgrade of "
+                                  "%(module)s.%(name)s: %(err)s",
+                                  module=participant.__class__.__module__,
+                                  name=participant.__class__.__name__,
+                                  err=exception_to_unicode(e)))
         return False
 
     def upgrade(self, backup=False, backup_dest=None):
@@ -857 +864 @@
                 CacheManager(env).reset_metadata()
     else:
         env = Environment(env_path)
-        needs_upgrade = True
         try:
             needs_upgrade = env.needs_upgrade()
+        except TracError as e:
+            env.log.error("Exception caught while checking for upgrade: %s",
+                          exception_to_unicode(e))
+            raise
         except Exception as e:  # e.g. no database connection
             env.log.error("Exception caught while checking for upgrade: %s",
                           exception_to_unicode(e, traceback=True))
-        if needs_upgrade:
-            raise TracError(_('The Trac Environment needs to be upgraded. '
-                              'Run:\n\n  trac-admin "%(path)s" upgrade',
-                              path=env_path))
+            raise
+        else:
+            if needs_upgrade:
+                raise TracError(_('The Trac Environment needs to be upgraded. '
+                                  'Run:\n\n  trac-admin "%(path)s" upgrade',
+                                  path=env_path))
 
     return env
 

tracd message after the revised patch:

Trac Error

TracError: Unable to check for upgrade of tracdragdrop.web_ui.TracDragDropModule: TypeError: environment_needs_upgrade() takes exactly 2 arguments (1 given)

Logging after the revised patch:

2019-05-28 21:45:16,920 Trac[env] ERROR: Component <Component tracdragdrop.web_ui.TracDragDropModule> failed with
Traceback (most recent call last):
  File "/home/jun66j5/src/tracdev/git/trac/env.py", line 387, in component_guard
    yield
  File "/home/jun66j5/src/tracdev/git/trac/env.py", line 750, in needs_upgrade
    if participant.environment_needs_upgrade():
TypeError: environment_needs_upgrade() takes exactly 2 arguments (1 given)
2019-05-28 21:45:16,924 Trac[env] ERROR: Exception caught while checking for upgrade: TracError: Unable to check for upgrade of tracdragdrop.web_ui.TracDragDropModule: TypeError: environment_needs_upgrade() takes exactly 2 arguments (1 given)

[Ryan J Ollos]

Looks like a good change.

[Jun Omae]

Thanks. Applied the patch in comment:2 in [16953].