fix: replace bare except clauses with except Exception#33426
fix: replace bare except clauses with except Exception#33426mango766 wants to merge 1 commit intolanggenius:mainfrom
Conversation
Bare `except:` catches all BaseException subclasses including SystemExit, KeyboardInterrupt, and GeneratorExit, which can prevent graceful process shutdown and mask configuration errors. Replace all bare except clauses in production code with `except Exception:` to narrow the catch scope while preserving existing behavior. Co-Authored-By: Claude (claude-opus-4-6) <noreply@anthropic.com>
mango766
requested review from
JohnJyong,
Mairuis,
QuantumGhost and
Yeuoly
as code owners
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors exception handling by replacing bare Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly replaces bare except: clauses with except Exception:, which is a good improvement as it prevents catching system-exiting exceptions. I've provided several suggestions to further improve the exception handling by catching more specific exceptions where possible. This will make the code more robust, easier to debug, and clearer in its intent.
| try: | ||
| valid_password(new_password) | ||
| except: | ||
| except Exception: |
There was a problem hiding this comment.
While except Exception: is better than a bare except:, we can be more specific here. The valid_password function raises a ValueError on failure. Catching ValueError specifically makes the code clearer and safer.
| except Exception: | |
| except ValueError: |
| try: | ||
| email_validate(normalized_new_email) | ||
| except: | ||
| except Exception: |
There was a problem hiding this comment.
Similar to the password validation, email_validate raises a ValueError for invalid emails. It's better to catch this specific exception.
| except Exception: | |
| except ValueError: |
| json_blocks.append(json.loads(json_text, strict=False)) | ||
| return json_blocks | ||
| except: | ||
| except Exception: |
There was a problem hiding this comment.
The primary exception expected in this try block is from json.loads(). It's good practice to catch the specific json.JSONDecodeError rather than a broad Exception. This makes the code's intent clearer.
| except Exception: | |
| except json.JSONDecodeError: |
| from alibabacloud_gpdb20160503.client import Client # type: ignore | ||
| from alibabacloud_tea_openapi import models as open_api_models # type: ignore | ||
| except: | ||
| except Exception: |
There was a problem hiding this comment.
This try...except block is intended to handle missing optional dependencies. The specific exception raised in this case is ImportError. It's better to catch ImportError explicitly.
| except Exception: | |
| except ImportError: |
| self._client.get(index=self._collection_name, id=id, params=params) | ||
| return True | ||
| except: | ||
| except Exception: |
There was a problem hiding this comment.
The text_exists method returns False if the document is not found. The self._client.get() call will raise a NotFoundError in this case. Catching this specific exception is more precise than catching a general Exception, as it avoids masking other potential issues like connection errors. You'll also need to add from opensearchpy.exceptions import NotFoundError.
| except Exception: | |
| except NotFoundError: |
| self._client.get(index=self._collection_name.lower(), id=id) | ||
| return True | ||
| except: | ||
| except Exception: |
There was a problem hiding this comment.
Similar to the Lindorm vector store, self._client.get() will raise NotFoundError if the document doesn't exist. Catching this specific exception is better than a broad Exception as it makes the code's behavior more predictable and avoids hiding other errors like network issues. Remember to import NotFoundError from opensearchpy.exceptions.
| except Exception: | |
| except NotFoundError: |
| except Exception: | ||
| return False |
There was a problem hiding this comment.
The head_object call raises a ClientError when an object is not found (404). Catching a generic Exception can mask other important errors, such as invalid credentials or network issues. It's better to specifically catch ClientError and check for the '404' error code. This makes the exists method more robust. You'll need to import ClientError from botocore.exceptions.
| except Exception: | |
| return False | |
| except ClientError as e: | |
| if e.response['Error']['Code'] == '404': | |
| return False | |
| else: | |
| # Reraise other client errors | |
| raise |
| except Exception: | ||
| return False |
There was a problem hiding this comment.
This client behaves similarly to the AWS S3 client. The head_object call will raise a ClientError for a missing object. To make this method more robust and avoid hiding other potential errors (like configuration or permission issues), it's better to specifically catch ClientError and check for the '404' error code. Remember to import ClientError from botocore.exceptions.
| except Exception: | |
| return False | |
| except ClientError as e: | |
| if e.response['Error']['Code'] == '404': | |
| return False | |
| else: | |
| # Reraise other client errors | |
| raise |
| raise ValueError(f"provider not found for tool {tool_name}") | ||
| meta["tool_icons"][tool_name] = json.loads(provider.icon) | ||
| except: | ||
| except Exception: |
There was a problem hiding this comment.
This try block can raise a ValueError (if the provider is not found) or a json.JSONDecodeError (if the icon data is invalid). It's better to catch these specific exceptions rather than a broad Exception. This makes the code's intent clearer and prevents accidentally catching unrelated errors.
| except Exception: | |
| except (ValueError, json.JSONDecodeError): |
dosubot
bot
added
the
size:S
1 participant
Replaces bare
except:clauses withexcept Exception:across multiple files in the API layer.Bare
except:catchesSystemExit,KeyboardInterrupt, andGeneratorExitin addition to regular exceptions, which can prevent graceful process shutdown and mask configuration errors.This is a mechanical change — the exception handling logic remains the same, only the catch scope is narrowed to exclude base exceptions.
Closes #33425