Skip to content

Conversation

@Wvidit
Copy link

@Wvidit Wvidit commented Feb 11, 2026

Description

Brief description of the changes in this PR.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)

Related Issues

Fixes #4446

Changes Made

  • Added logging for some JSON parsing errors

Testing

Describe the tests you ran to verify your changes:

  • Unit tests pass (cd core && pytest tests/)
  • Lint passes (cd core && ruff check .)
  • Manual testing performed

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@Lumi-artLife
Copy link

Good fix! Silent JSON parse errors are frustrating to debug. Adding logging here will help developers identify config issues much faster.

One question: Are you logging just the error message, or also the problematic JSON content (truncated for privacy)? Sometimes seeing the actual malformed JSON helps pinpoint the issue, especially if it's a syntax error near a specific character.

Also, consider the log level - get_hive_config() is called frequently, so DEBUG level might be appropriate to avoid flooding logs in production.

@Wvidit
Copy link
Author

Wvidit commented Feb 11, 2026

Problematic JSON config file path along with the error occurred are being logged, thanks for pointing out the overflow issue.

I was thinking of adding a simple cache for it.

Copy link
Collaborator

@bryanadenhq bryanadenhq left a comment

Choose a reason for hiding this comment

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

Hi @Wvidit, the logging changes look good, but there are two things that need fixing before megre, the lru_cache introduces more issues than it solves so we should just remove it for now, and there seems to be a merge conflict. Thanks for your contribution!

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.

bug(config): get_hive_config() silently swallows JSON parse errors

3 participants