diff --git a/fire/completion.py b/fire/completion.py index 1597d464..b5460b34 100644 --- a/fire/completion.py +++ b/fire/completion.py @@ -359,7 +359,7 @@ def VisibleMembers(component, class_attrs=None, verbose=False): if isinstance(component, dict): members = component.items() else: - members = inspect.getmembers(component) + members = inspectutils.GetMembers(component) # If class_attrs has not been provided, compute it. if class_attrs is None: diff --git a/fire/core.py b/fire/core.py index 8e23e76b..0bde49d1 100644 --- a/fire/core.py +++ b/fire/core.py @@ -225,7 +225,7 @@ def _IsHelpShortcut(component_trace, remaining_args): _, remaining_kwargs, _ = _ParseKeywordArgs(remaining_args, fn_spec) show_help = target in remaining_kwargs else: - members = dict(inspect.getmembers(component)) + members = dict(inspectutils.GetMembers(component)) show_help = target not in members if show_help: diff --git a/fire/fire_test.py b/fire/fire_test.py index 99b4a7c6..ae8bc90d 100644 --- a/fire/fire_test.py +++ b/fire/fire_test.py @@ -556,6 +556,35 @@ def testHelpFlagAndTraceFlag(self): with self.assertRaisesFireExit(0, 'Fire trace:\n.*SYNOPSIS'): fire.Fire(tc.BoolConverter, command=['--', '-h', '--trace']) + def testHelpWithRaisingProperty(self): + # A property whose getter raises should not prevent help from being shown. + # See https://github.com/google/python-fire/issues/672. + class Component(object): + + @property + def broken(self): + raise RuntimeError('backend unavailable') + + def works(self, value): + return value + + component = Component() + + # Bare invocation prints the component's help without crashing. + with self.assertOutputMatches(stdout='.*works.*'): + result = fire.Fire(component, command=[]) + self.assertEqual(result, component) + + # Both the --help flag and the --help shortcut list the working command. + with self.assertRaisesFireExit(0, 'works'): + fire.Fire(component, command=['--', '--help']) + with self.assertRaisesFireExit(0, 'works'): + fire.Fire(component, command=['--help']) + + # The broken property is still usable directly, but raises on access. + with self.assertRaises(RuntimeError): + component.broken # pylint: disable=pointless-statement + def testTabCompletionNoName(self): completion_script = fire.Fire(tc.NoDefaults, command=['--', '--completion']) self.assertIn('double', completion_script) diff --git a/fire/inspectutils.py b/fire/inspectutils.py index 17508e30..4e5b654e 100644 --- a/fire/inspectutils.py +++ b/fire/inspectutils.py @@ -342,6 +342,33 @@ def GetClassAttrsDict(component): } +def GetMembers(component): + """Returns a list of (name, member) pairs for the members of component. + + Reading a member can run arbitrary code, such as a property getter, which may + raise an exception. inspect.getmembers aborts on any exception that is not an + AttributeError, which would crash Fire while it only intends to enumerate the + members of the component, for example to build help text. This falls back to + reading the members one at a time and skipping any that cannot be read, so a + single broken member does not prevent the rest from being listed. + + Args: + component: The object whose members to list. + Returns: + A list of (name, value) pairs for the readable members of component. + """ + try: + return inspect.getmembers(component) + except Exception: # pylint: disable=broad-except + members = [] + for name in dir(component): + try: + members.append((name, getattr(component, name))) + except Exception: # pylint: disable=broad-except + continue + return members + + def IsCoroutineFunction(fn): try: return inspect.iscoroutinefunction(fn)