Skip to content

Commit f02b3ce

Browse files
[FSSDK-12497] Fix return in finally block silently swallowing exceptions (#505)
Move error-handling logic out of finally block into normal control flow and change bare except to except Exception. Per Python docs, a return in finally suppresses any in-flight exception, which silently swallows errors raised inside except blocks. Fixes #439 Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 54ae4c6 commit f02b3ce

File tree

2 files changed

+36
-6
lines changed

2 files changed

+36
-6
lines changed

optimizely/config_manager.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -145,14 +145,14 @@ def _set_config(self, datafile: Optional[str | bytes]) -> None:
145145
except optimizely_exceptions.UnsupportedDatafileVersionException as error:
146146
error_msg = error.args[0]
147147
error_to_handle = error
148-
except:
148+
except Exception:
149149
error_msg = enums.Errors.INVALID_INPUT.format('datafile')
150150
error_to_handle = optimizely_exceptions.InvalidInputException(error_msg)
151-
finally:
152-
if error_msg or config is None:
153-
self.logger.error(error_msg)
154-
self.error_handler.handle_error(error_to_handle or Exception('Unknown Error'))
155-
return
151+
152+
if error_msg or config is None:
153+
self.logger.error(error_msg)
154+
self.error_handler.handle_error(error_to_handle or Exception('Unknown Error'))
155+
return
156156

157157
previous_revision = self._config.get_revision() if self._config else None
158158

tests/test_config_manager.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,36 @@ def test_set_config__invalid_datafile(self):
200200
mock_logger.error.assert_called_once_with('Provided "datafile" is in an invalid format.')
201201
self.assertEqual(0, mock_notification_center.call_count)
202202

203+
def test_set_config__exception_in_except_block_is_not_swallowed(self):
204+
""" Test that an exception raised inside the except block propagates
205+
to the caller instead of being silently swallowed.
206+
207+
Per Python docs, a return inside a finally clause suppresses any
208+
in-flight exception. The _set_config method must not use return in
209+
finally, so that unexpected errors in the error-handling path are
210+
surfaced rather than silently lost.
211+
"""
212+
test_datafile = json.dumps(self.config_dict_with_features)
213+
mock_logger = mock.Mock()
214+
215+
with mock.patch('optimizely.config_manager.BaseConfigManager._validate_instantiation_options'):
216+
project_config_manager = config_manager.StaticConfigManager(
217+
datafile=test_datafile, logger=mock_logger,
218+
)
219+
220+
# Make ProjectConfig raise so we enter the bare except block,
221+
# then make the except block itself raise by breaking INVALID_INPUT.
222+
with mock.patch(
223+
'optimizely.project_config.ProjectConfig.__init__',
224+
side_effect=Exception('something broke'),
225+
), mock.patch.object(
226+
enums.Errors, 'INVALID_INPUT', new=None,
227+
):
228+
# The except block will raise AttributeError ('NoneType' has no 'format').
229+
# Correct behavior: this exception must propagate to the caller.
230+
with self.assertRaises(AttributeError):
231+
project_config_manager._set_config(test_datafile)
232+
203233
def test_get_config(self):
204234
""" Test get_config. """
205235
test_datafile = json.dumps(self.config_dict_with_features)

0 commit comments

Comments
 (0)