Skip to content

Activate rules to prevent exception handling anti-patterns#11282

Merged
lukasmasuch merged 4 commits intodevelopfrom
activate-try-linting-rule
May 6, 2025
Merged

Activate rules to prevent exception handling anti-patterns#11282
lukasmasuch merged 4 commits intodevelopfrom
activate-try-linting-rule

Conversation

@lukasmasuch
Copy link
Copy Markdown
Collaborator

@lukasmasuch lukasmasuch commented May 6, 2025

Describe your changes

Activates linting rules to prevent exception handling anti-patterns in Python:


Contribution License Agreement

By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.

@lukasmasuch lukasmasuch changed the title Activate rules to prevent exception handling anti-patterns [WIP] Activate rules to prevent exception handling anti-patterns May 6, 2025
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented May 6, 2025

✅ PR preview is ready!

Name Link
📦 Wheel file https://core-previews.s3-us-west-2.amazonaws.com/pr-11282/streamlit-1.45.0-py3-none-any.whl
🕹️ Preview app pr-11282.streamlit.app (☁️ Deploy here if not accessible)

@lukasmasuch lukasmasuch added security-assessment-completed change:other PR contains other type of change impact:internal PR changes only affect internal code labels May 6, 2025
lukasmasuch and others added 2 commits May 6, 2025 16:16
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@sfc-gh-lmasuch sfc-gh-lmasuch changed the title [WIP] Activate rules to prevent exception handling anti-patterns Activate rules to prevent exception handling anti-patterns May 6, 2025
@sfc-gh-lmasuch sfc-gh-lmasuch marked this pull request as ready for review May 6, 2025 14:19
@sfc-gh-lmasuch sfc-gh-lmasuch requested a review from a team as a code owner May 6, 2025 14:19
@lukasmasuch lukasmasuch requested a review from Copilot May 6, 2025 14:41
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR activates additional linting rules aimed at preventing exception handling anti‐patterns throughout the codebase. Key changes include switching from _LOGGER.error to _LOGGER.exception for improved logging of exceptions, standardizing exception types (e.g. using RuntimeError or TypeError), and updating linter configurations in .ruff.toml.

  • Updated logging calls across multiple server and connection modules.
  • Revised exception raises to use more specific exception types.
  • Refined linter ignore settings in .ruff.toml to enforce the new TRY lint rules.

Reviewed Changes

Copilot reviewed 29 out of 29 changed files in this pull request and generated no comments.

Show a summary per file
File Description
lib/streamlit/web/server/media_file_handler.py Changed logging from _LOGGER.error to _LOGGER.exception for missing file errors.
lib/streamlit/web/server/component_request_handler.py Updated exception logging and error sanitization during file read errors.
lib/streamlit/web/server/browser_websocket_handler.py Replaced RuntimeError with TypeError for incorrect message payload types.
lib/streamlit/web/bootstrap.py Switched error logging to _LOGGER.exception when loading secrets.
lib/streamlit/runtime/scriptrunner/script_runner.py Updated exception type from Exception to RuntimeError and simplified re-raising exceptions.
lib/streamlit/runtime/caching/storage/cache_storage_protocol.py Renamed InvalidCacheStorageContext to InvalidCacheStorageContextError.
lib/streamlit/connections/snowflake_connection.py Adjusted exception handling for Snowflake connection errors.
.ruff.toml Added new TRY lint rules and updated per-file ignore patterns to accommodate them.
Comments suppressed due to low confidence (3)

lib/streamlit/web/server/media_file_handler.py:90

  • Ensure that using _LOGGER.exception here provides sufficient context for troubleshooting without exposing sensitive internal details. Verify that logging the full stack trace is appropriate for the production environment.
_LOGGER.exception("MediaFileHandler: Missing file %s", absolute_path)

lib/streamlit/web/server/browser_websocket_handler.py:215

  • Ensure that replacing RuntimeError with TypeError accurately reflects the type-checking failure and that the exception message clearly indicates that string messages are not supported.
raise TypeError(  # noqa: TRY301

.ruff.toml:122

  • [nitpick] Review and update the .ruff.toml configuration to ensure that the new TRY lint rules integrate well with existing exception logging patterns and do not cause unintended false positives.
"TRY003",  # Checks for exception messages that are not defined in the exception class itself.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented May 6, 2025

📉 Python coverage change detected

The Python unit test coverage has decreased by 0.0330%

  • Current PR: 92.1214% (17922 statements, 1412 missed)
  • Latest develop: 92.1545% (17921 statements, 1406 missed)

💡 Consider adding more unit tests to maintain or improve coverage.

Coverage by files
Name Stmts Miss Cover
streamlit/__init__.py 139 0 100%
streamlit/__main__.py 3 3 0%
streamlit/auth_util.py 100 25 75%
streamlit/cli_util.py 38 6 84%
streamlit/column_config.py 3 0 100%
streamlit/commands/__init__.py 0 0 100%
streamlit/commands/echo.py 54 11 80%
streamlit/commands/execution_control.py 58 19 67%
streamlit/commands/experimental_query_params.py 40 2 95%
streamlit/commands/logo.py 40 6 85%
streamlit/commands/navigation.py 104 2 98%
streamlit/commands/page_config.py 97 4 96%
streamlit/components/__init__.py 0 0 100%
streamlit/components/lib/__init__.py 0 0 100%
streamlit/components/lib/local_component_registry.py 35 2 94%
streamlit/components/types/__init__.py 0 0 100%
streamlit/components/types/base_component_registry.py 14 0 100%
streamlit/components/types/base_custom_component.py 47 5 89%
streamlit/components/v1/__init__.py 5 0 100%
streamlit/components/v1/component_arrow.py 33 8 76%
streamlit/components/v1/component_registry.py 35 3 91%
streamlit/components/v1/components.py 4 4 0%
streamlit/components/v1/custom_component.py 91 7 92%
streamlit/config.py 307 8 97%
streamlit/config_option.py 77 1 99%
streamlit/config_util.py 76 5 93%
streamlit/connections/__init__.py 6 0 100%
streamlit/connections/base_connection.py 45 0 100%
streamlit/connections/snowflake_connection.py 60 13 78%
streamlit/connections/snowpark_connection.py 44 3 93%
streamlit/connections/sql_connection.py 55 6 89%
streamlit/connections/util.py 33 0 100%
streamlit/cursor.py 82 2 98%
streamlit/dataframe_util.py 506 45 91%
streamlit/delta_generator.py 204 5 98%
streamlit/delta_generator_singletons.py 76 8 89%
streamlit/deprecation_util.py 57 4 93%
streamlit/development.py 1 0 100%
streamlit/elements/__init__.py 0 0 100%
streamlit/elements/alert.py 60 0 100%
streamlit/elements/arrow.py 156 14 91%
streamlit/elements/balloons.py 10 0 100%
streamlit/elements/bokeh_chart.py 25 0 100%
streamlit/elements/code.py 21 1 95%
streamlit/elements/deck_gl_json_chart.py 97 7 93%
streamlit/elements/dialog_decorator.py 58 2 97%
streamlit/elements/doc_string.py 234 9 96%
streamlit/elements/empty.py 16 4 75%
streamlit/elements/exception.py 104 11 89%
streamlit/elements/form.py 46 2 96%
streamlit/elements/graphviz_chart.py 29 1 97%
streamlit/elements/heading.py 50 0 100%
streamlit/elements/html.py 45 0 100%
streamlit/elements/iframe.py 32 1 97%
streamlit/elements/image.py 32 1 97%
streamlit/elements/json.py 41 2 95%
streamlit/elements/layouts.py 106 3 97%
streamlit/elements/lib/__init__.py 0 0 100%
streamlit/elements/lib/built_in_chart_utils.py 361 33 91%
streamlit/elements/lib/color_util.py 102 4 96%
streamlit/elements/lib/column_config_utils.py 169 1 99%
streamlit/elements/lib/column_types.py 129 0 100%
streamlit/elements/lib/dialog.py 54 1 98%
streamlit/elements/lib/dicttools.py 39 2 95%
streamlit/elements/lib/event_utils.py 10 1 90%
streamlit/elements/lib/file_uploader_utils.py 22 0 100%
streamlit/elements/lib/form_utils.py 26 0 100%
streamlit/elements/lib/image_utils.py 177 24 86%
streamlit/elements/lib/js_number.py 28 3 89%
streamlit/elements/lib/layout_utils.py 16 1 94%
streamlit/elements/lib/mutable_status_container.py 70 3 96%
streamlit/elements/lib/options_selector_utils.py 98 0 100%
streamlit/elements/lib/pandas_styler_utils.py 72 1 99%
streamlit/elements/lib/policies.py 56 1 98%
streamlit/elements/lib/streamlit_plotly_theme.py 48 0 100%
streamlit/elements/lib/subtitle_utils.py 77 13 83%
streamlit/elements/lib/utils.py 72 5 93%
streamlit/elements/map.py 113 5 96%
streamlit/elements/markdown.py 54 2 96%
streamlit/elements/media.py 169 8 95%
streamlit/elements/metric.py 79 5 94%
streamlit/elements/plotly_chart.py 95 3 97%
streamlit/elements/progress.py 42 0 100%
streamlit/elements/pyplot.py 34 2 94%
streamlit/elements/snow.py 10 0 100%
streamlit/elements/spinner.py 31 0 100%
streamlit/elements/text.py 13 0 100%
streamlit/elements/toast.py 17 0 100%
streamlit/elements/vega_charts.py 196 3 98%
streamlit/elements/widgets/__init__.py 0 0 100%
streamlit/elements/widgets/audio_input.py 70 11 84%
streamlit/elements/widgets/button.py 196 45 77%
streamlit/elements/widgets/button_group.py 169 0 100%
streamlit/elements/widgets/camera_input.py 70 10 86%
streamlit/elements/widgets/chat.py 159 42 74%
streamlit/elements/widgets/checkbox.py 49 0 100%
streamlit/elements/widgets/color_picker.py 53 3 94%
streamlit/elements/widgets/data_editor.py 230 14 94%
streamlit/elements/widgets/file_uploader.py 113 19 83%
streamlit/elements/widgets/multiselect.py 105 4 96%
streamlit/elements/widgets/number_input.py 164 11 93%
streamlit/elements/widgets/radio.py 82 6 93%
streamlit/elements/widgets/select_slider.py 106 1 99%
streamlit/elements/widgets/selectbox.py 104 2 98%
streamlit/elements/widgets/slider.py 257 17 93%
streamlit/elements/widgets/text_widgets.py 140 7 95%
streamlit/elements/widgets/time_widgets.py 266 17 94%
streamlit/elements/write.py 173 30 83%
streamlit/emojis.py 4 0 100%
streamlit/env_util.py 25 5 80%
streamlit/error_util.py 32 2 94%
streamlit/errors.py 148 30 80%
streamlit/external/__init__.py 0 0 100%
streamlit/external/langchain/__init__.py 2 0 100%
streamlit/external/langchain/streamlit_callback_handler.py 140 81 42%
streamlit/file_util.py 82 7 91%
streamlit/git_util.py 97 60 38%
streamlit/logger.py 56 0 100%
streamlit/material_icon_names.py 1 0 100%
streamlit/navigation/__init__.py 0 0 100%
streamlit/navigation/page.py 76 2 97%
streamlit/net_util.py 55 3 95%
streamlit/platform.py 10 1 90%
streamlit/runtime/__init__.py 8 0 100%
streamlit/runtime/app_session.py 370 61 84%
streamlit/runtime/caching/__init__.py 19 0 100%
streamlit/runtime/caching/cache_data_api.py 166 3 98%
streamlit/runtime/caching/cache_errors.py 45 1 98%
streamlit/runtime/caching/cache_resource_api.py 126 0 100%
streamlit/runtime/caching/cache_type.py 11 1 91%
streamlit/runtime/caching/cache_utils.py 160 9 94%
streamlit/runtime/caching/cached_message_replay.py 103 2 98%
streamlit/runtime/caching/hashing.py 308 23 93%
streamlit/runtime/caching/legacy_cache_api.py 13 0 100%
streamlit/runtime/caching/storage/__init__.py 2 0 100%
streamlit/runtime/caching/storage/cache_storage_protocol.py 31 2 94%
streamlit/runtime/caching/storage/dummy_cache_storage.py 21 0 100%
streamlit/runtime/caching/storage/in_memory_cache_storage_wrapper.py 60 0 100%
streamlit/runtime/caching/storage/local_disk_cache_storage.py 86 4 95%
streamlit/runtime/connection_factory.py 85 9 89%
streamlit/runtime/context.py 126 35 72%
streamlit/runtime/context_util.py 18 0 100%
streamlit/runtime/credentials.py 142 9 94%
streamlit/runtime/forward_msg_cache.py 23 2 91%
streamlit/runtime/forward_msg_queue.py 63 4 94%
streamlit/runtime/fragment.py 123 1 99%
streamlit/runtime/media_file_manager.py 69 7 90%
streamlit/runtime/media_file_storage.py 15 0 100%
streamlit/runtime/memory_media_file_storage.py 70 0 100%
streamlit/runtime/memory_session_storage.py 15 0 100%
streamlit/runtime/memory_uploaded_file_manager.py 41 1 98%
streamlit/runtime/metrics_util.py 194 12 94%
streamlit/runtime/pages_manager.py 59 2 97%
streamlit/runtime/runtime.py 234 16 93%
streamlit/runtime/runtime_util.py 30 1 97%
streamlit/runtime/script_data.py 16 0 100%
streamlit/runtime/scriptrunner/__init__.py 5 0 100%
streamlit/runtime/scriptrunner/exec_code.py 49 5 90%
streamlit/runtime/scriptrunner/magic.py 83 1 99%
streamlit/runtime/scriptrunner/magic_funcs.py 10 1 90%
streamlit/runtime/scriptrunner/script_cache.py 27 0 100%
streamlit/runtime/scriptrunner/script_runner.py 224 20 91%
streamlit/runtime/scriptrunner_utils/__init__.py 0 0 100%
streamlit/runtime/scriptrunner_utils/exceptions.py 11 1 91%
streamlit/runtime/scriptrunner_utils/script_requests.py 105 4 96%
streamlit/runtime/scriptrunner_utils/script_run_context.py 141 2 99%
streamlit/runtime/secrets.py 243 26 89%
streamlit/runtime/session_manager.py 59 0 100%
streamlit/runtime/state/__init__.py 7 0 100%
streamlit/runtime/state/common.py 49 2 96%
streamlit/runtime/state/query_params.py 109 3 97%
streamlit/runtime/state/query_params_proxy.py 81 0 100%
streamlit/runtime/state/safe_session_state.py 73 14 81%
streamlit/runtime/state/session_state.py 361 13 96%
streamlit/runtime/state/session_state_proxy.py 62 8 87%
streamlit/runtime/state/widgets.py 12 1 92%
streamlit/runtime/stats.py 42 0 100%
streamlit/runtime/uploaded_file_manager.py 37 2 95%
streamlit/runtime/websocket_session_manager.py 60 0 100%
streamlit/source_util.py 35 1 97%
streamlit/string_util.py 77 2 97%
streamlit/temporary_directory.py 15 1 93%
streamlit/testing/__init__.py 0 0 100%
streamlit/testing/v1/__init__.py 2 0 100%
streamlit/testing/v1/app_test.py 239 6 97%
streamlit/testing/v1/element_tree.py 1330 84 94%
streamlit/testing/v1/local_script_runner.py 73 0 100%
streamlit/testing/v1/util.py 17 0 100%
streamlit/time_util.py 32 1 97%
streamlit/type_util.py 145 9 94%
streamlit/url_util.py 40 5 88%
streamlit/user_info.py 87 8 91%
streamlit/util.py 26 0 100%
streamlit/version.py 3 0 100%
streamlit/watcher/__init__.py 3 0 100%
streamlit/watcher/event_based_path_watcher.py 156 20 87%
streamlit/watcher/folder_black_list.py 14 1 93%
streamlit/watcher/local_sources_watcher.py 125 8 94%
streamlit/watcher/path_watcher.py 45 3 93%
streamlit/watcher/polling_path_watcher.py 49 2 96%
streamlit/watcher/util.py 52 1 98%
streamlit/web/__init__.py 0 0 100%
streamlit/web/bootstrap.py 145 18 88%
streamlit/web/cache_storage_manager_config.py 5 0 100%
streamlit/web/cli.py 176 16 91%
streamlit/web/server/__init__.py 5 0 100%
streamlit/web/server/app_static_file_handler.py 29 3 90%
streamlit/web/server/authlib_tornado_integration.py 18 1 94%
streamlit/web/server/browser_websocket_handler.py 109 27 75%
streamlit/web/server/component_request_handler.py 62 6 90%
streamlit/web/server/media_file_handler.py 62 9 85%
streamlit/web/server/oauth_authlib_routes.py 116 16 86%
streamlit/web/server/oidc_mixin.py 45 0 100%
streamlit/web/server/routes.py 81 7 91%
streamlit/web/server/server.py 177 11 94%
streamlit/web/server/server_util.py 60 8 87%
streamlit/web/server/stats_request_handler.py 51 2 96%
streamlit/web/server/upload_file_request_handler.py 51 11 78%
streamlit/web/server/websocket_headers.py 19 1 95%
TOTAL 17922 1412 92%

📊 View detailed coverage comparison

@snyk-io
Copy link
Copy Markdown
Contributor

snyk-io bot commented May 6, 2025

🎉 Snyk checks have passed. No issues have been found so far.

security/snyk check is complete. No issues have been found. (View Details)

license/snyk check is complete. No issues have been found. (View Details)

@lukasmasuch lukasmasuch merged commit b409db7 into develop May 6, 2025
37 checks passed
@lukasmasuch lukasmasuch deleted the activate-try-linting-rule branch May 6, 2025 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:other PR contains other type of change impact:internal PR changes only affect internal code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants