Skip to content

Commit 3e22fe1

Browse files
authored
Merge pull request #92 from Miyamura80/claude/fix-todo-issues-PWUug
Fix exception handling and validator anti-patterns
2 parents f963c8d + 0ccfdb9 commit 3e22fe1

5 files changed

Lines changed: 22 additions & 31 deletions

File tree

TODO.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@
99

1010
### High Priority
1111

12-
- [ ] Broad exception handling in `src/utils/logging_config.py:64` - catches all exceptions indiscriminately
13-
- [ ] Validator anti-pattern in `common/global_config.py:188-198` - validators ignore input parameter `v`
14-
- [ ] Circular import risk in `common/flags.py:6-7` - `setup_logging()` called at import time
15-
- [ ] Broad exceptions in `utils/llm/dspy_langfuse.py:280,321,438` - catches generic `Exception`
16-
- [ ] Unsafe exception re-instantiation in `src/utils/logging_config.py:61-71` - reconstructs exceptions unsafely
12+
- [x] Broad exception handling in `src/utils/logging_config.py:64` - catches all exceptions indiscriminately
13+
- [x] Validator anti-pattern in `common/global_config.py:188-198` - validators ignore input parameter `v`
14+
- [x] Circular import risk in `common/flags.py:6-7` - `setup_logging()` called at import time
15+
- [x] Broad exceptions in `utils/llm/dspy_langfuse.py:280,321,438` - catches generic `Exception`
16+
- [x] Unsafe exception re-instantiation in `src/utils/logging_config.py:61-71` - reconstructs exceptions unsafely
1717

1818
### Medium Priority
1919

common/flags.py

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,6 @@
33
from openfeature.provider.in_memory_provider import InMemoryProvider
44

55
from common.global_config import global_config
6-
from src.utils.logging_config import setup_logging
7-
8-
setup_logging()
96

107

118
def setup_feature_flags():

common/global_config.py

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
import yaml
88
from dotenv import dotenv_values, load_dotenv
99
from loguru import logger
10-
from pydantic import Field, field_validator
10+
from pydantic import Field
1111
from pydantic_settings import (
1212
BaseSettings,
1313
PydanticBaseSettingsSource,
@@ -180,22 +180,15 @@ class Config(BaseSettings):
180180
PERPLEXITY_API_KEY: str | None = None
181181
GEMINI_API_KEY: str | None = None
182182

183-
# Runtime environment (computed)
184-
is_local: bool = Field(default=False)
185-
running_on: str = Field(default="")
186-
187-
@field_validator("is_local", mode="before")
188-
@classmethod
189-
def set_is_local(cls, v: Any) -> bool:
190-
"""Set is_local based on GITHUB_ACTIONS env var."""
191-
return os.getenv("GITHUB_ACTIONS") != "true"
192-
193-
@field_validator("running_on", mode="before")
194-
@classmethod
195-
def set_running_on(cls, v: Any) -> str:
196-
"""Set running_on based on is_local."""
197-
is_local = os.getenv("GITHUB_ACTIONS") != "true"
198-
return "🖥️ local" if is_local else "☁️ CI"
183+
# Runtime environment (computed via default_factory)
184+
is_local: bool = Field(
185+
default_factory=lambda: os.getenv("GITHUB_ACTIONS") != "true"
186+
)
187+
running_on: str = Field(
188+
default_factory=lambda: "🖥️ local"
189+
if os.getenv("GITHUB_ACTIONS") != "true"
190+
else "☁️ CI"
191+
)
199192

200193
@classmethod
201194
def settings_customise_sources(

src/utils/logging_config.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,10 @@ def scrub_sensitive_data(record):
9595
try:
9696
# Most standard exceptions accept a single string argument
9797
new_value = type_(scrubbed_value_str)
98-
except Exception:
98+
except TypeError:
9999
# Fallback to a generic Exception if type instantiation fails
100-
new_value = Exception(scrubbed_value_str)
100+
# (e.g., some exceptions require specific constructor arguments)
101+
new_value = Exception(f"[{type_.__name__}] {scrubbed_value_str}")
101102

102103
# Preserve traceback and context metadata
103104
new_value.__traceback__ = tb

utils/llm/dspy_langfuse.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ def on_lm_end( # noqa
277277
prompt=current_system_prompt + current_prompt,
278278
completion=current_completion_content,
279279
)
280-
except Exception as cost_calc_exception:
280+
except (ValueError, KeyError, TypeError) as cost_calc_exception:
281281
log.warning(
282282
f"litellm.completion_cost failed for model {current_model_name}: {cost_calc_exception}"
283283
)
@@ -318,8 +318,8 @@ def on_lm_end( # noqa
318318
usage_details=usage_details_update,
319319
cost_details=cost_details_update,
320320
)
321-
except Exception as e:
322-
# This outer try-except catches errors in the token calculation logic itself
321+
except (TypeError, ValueError, ZeroDivisionError, AttributeError) as e:
322+
# This try-except catches errors in the token calculation logic itself
323323
log.warning(f"General failure in usage/cost block: {str(e)}")
324324
if level == "DEFAULT":
325325
level = "WARNING"
@@ -435,7 +435,7 @@ def on_tool_end( # noqa
435435
output_value = outputs.__dict__
436436
else:
437437
output_value = str(outputs)
438-
except Exception as e:
438+
except (TypeError, AttributeError, RecursionError) as e:
439439
output_value = {"serialization_error": str(e), "raw": str(outputs)}
440440

441441
tool_span.end(

0 commit comments

Comments
 (0)