Skip to content

Fix broad exception handling in CLI step lookup and resume run-id validation#3179

Merged
saikonen merged 1 commit into
Netflix:masterfrom
LuisJG8:fix/cli-staged-cleanups
Jun 5, 2026
Merged

Fix broad exception handling in CLI step lookup and resume run-id validation#3179
saikonen merged 1 commit into
Netflix:masterfrom
LuisJG8:fix/cli-staged-cleanups

Conversation

@LuisJG8

@LuisJG8 LuisJG8 commented May 6, 2026

Copy link
Copy Markdown
Contributor

PR Type

  • Bug fix

Summary

This PR fixes CLI error-handling behavior in two command paths.

In metaflow/cli_components/step_cmd.py, the step lookup now catches only AttributeError when a step is missing, instead of a bare except. In metaflow/cli_components/run_cmds.py, run_id validation now catches only TypeError and ValueError when checking whether the value is an integer.

I added these changes to prevent unrelated internal exceptions from being swallowed and reported as user-facing command errors, while preserving existing CLI behavior for valid and invalid inputs.

@greptile-apps

greptile-apps Bot commented May 6, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR tightens two overly broad except: clauses in CLI command handlers, replacing them with the specific exception types each code path is actually expected to raise.

  • In run_cmds.py, the int(run_id) conversion now catches only TypeError and ValueError, which are the only exceptions that can realistically arise when converting a CLI string argument to an integer.
  • In step_cmd.py, the getattr call now catches only AttributeError, which is the precise exception raised when a flow attribute (step) does not exist.

Confidence Score: 5/5

Safe to merge — both changes are minimal, targeted narrowings of exception scope with no behavioral change on the happy path.

Both changes correctly identify the expected exceptions for their respective operations: ValueError/TypeError for int() conversion of a string, and AttributeError for a missing getattr result. No logic is altered, no new code paths are introduced, and the fix prevents real internal exceptions from being silently swallowed and misreported as user-facing CLI errors.

No files require special attention.

Important Files Changed

Filename Overview
metaflow/cli_components/run_cmds.py Narrows bare except: to except (TypeError, ValueError): when calling int(run_id) — correct set of exceptions for integer conversion of a CLI string argument.
metaflow/cli_components/step_cmd.py Narrows bare except: to except AttributeError: on getattr call — correctly targets the only exception getattr raises when an attribute is absent.

Reviews (1): Last reviewed commit: "Narrow CLI exception handling in step/re..." | Re-trigger Greptile

@codecov

codecov Bot commented Jun 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (master@08b7218). Learn more about missing BASE report.

Files with missing lines Patch % Lines
metaflow/cli_components/run_cmds.py 0.00% 1 Missing ⚠️
metaflow/cli_components/step_cmd.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             master    #3179   +/-   ##
=========================================
  Coverage          ?   28.31%           
=========================================
  Files             ?      381           
  Lines             ?    52346           
  Branches          ?     9238           
=========================================
  Hits              ?    14821           
  Misses            ?    36585           
  Partials          ?      940           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@saikonen
saikonen merged commit c7711e3 into Netflix:master Jun 5, 2026
41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants