Skip to content

log failed expr compilations#6528

Merged
dogancanbakir merged 1 commit intodevfrom
6523_make_verbose
Oct 14, 2025
Merged

log failed expr compilations#6528
dogancanbakir merged 1 commit intodevfrom
6523_make_verbose

Conversation

@dogancanbakir
Copy link
Member

@dogancanbakir dogancanbakir commented Oct 13, 2025

Proposed changes

close #6523

Checklist

  • Pull request is created against the dev branch
  • All checks passed (lint, unit/integration/regression tests etc.) with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Summary by CodeRabbit

  • Chores
    • Added warning-level logs when expression compilation or evaluation fails, improving visibility into issues during runtime.
    • Log messages include the affected expression and error details to speed up troubleshooting.
    • Standardized logging output for consistency across diagnostics.
    • No change to normal behavior or outcomes; only additional warnings are emitted when errors occur.

@dogancanbakir dogancanbakir self-assigned this Oct 13, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 13, 2025

Walkthrough

Introduces warning logs in expression compilation and evaluation within pkg/protocols/common/expressions/expressions.go using gologger. On failures during compile or evaluate, a warning with the expression and error is emitted. No exported API changes or control-flow alterations beyond added logging side effects.

Changes

Cohort / File(s) Summary
Logging integration in expressions
pkg/protocols/common/expressions/expressions.go
Imported gologger and added warning logs on expression compile/evaluate failures without changing external behavior or exported APIs.

Sequence Diagram(s)

sequenceDiagram
  actor User
  participant Runner as TemplateRunner
  participant Expr as Expressions
  participant Log as gologger

  User->>Runner: Run template
  Runner->>Expr: Compile(expression)
  alt compile success
    Expr-->>Runner: compiledExpr
    Runner->>Expr: Evaluate(compiledExpr, base)
    alt evaluate success
      Expr-->>Runner: result
    else evaluate failure
      Expr->>Log: warn("Failed to evaluate expression '%s': %v")
      Expr-->>Runner: continue
    end
  else compile failure
    Expr->>Log: warn("Failed to compile expression '%s': %v")
    Expr-->>Runner: continue
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I twitch my whiskers at the screen,
Logs now whisper where errors have been.
Compile? Evaluate? If they fall—ding!
Warnings hop out, clear as spring.
Fewer mysteries in the burrow tonight—
Debugging made light, by carrot-bright insight. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly describes the primary change of adding logs for failed expression compilations and is concise and directly related to the PR’s main change.
Linked Issues Check ✅ Passed The change adds gologger calls to log warnings for both expression compilation and evaluation failures exactly as requested by issue #6523, satisfying the requirement to surface errors during expression handling.
Out of Scope Changes Check ✅ Passed The pull request only adds logging for compile and evaluation failures and imports gologger as required by the linked issue, and does not include any unrelated or extraneous changes.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 6523_make_verbose

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
pkg/protocols/common/expressions/expressions.go (1)

63-67: LGTM! Evaluation error logging completes the improvement.

The warning log mirrors the compilation error handling pattern, ensuring both failure scenarios are now visible. This directly fulfills the issue requirements for improved debugging.

Optional enhancement: Consider enriching the log messages with additional context (e.g., template name or target URL) if available in the call chain. This would further improve debugging when multiple templates or targets are involved. However, this can be deferred as a future improvement since the current change already provides significant value.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4e2af6b and cc1d646.

📒 Files selected for processing (1)
  • pkg/protocols/common/expressions/expressions.go (2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.go: Format Go code using go fmt
Run static analysis with go vet

Files:

  • pkg/protocols/common/expressions/expressions.go
pkg/protocols/**/*.go

📄 CodeRabbit inference engine (CLAUDE.md)

Each protocol implementation must provide a Request interface with methods Compile(), ExecuteWithResults(), Match(), and Extract()

Files:

  • pkg/protocols/common/expressions/expressions.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Lint
🔇 Additional comments (2)
pkg/protocols/common/expressions/expressions.go (2)

7-7: LGTM! Import addition is appropriate.

The gologger import is correctly added to support the new warning logs for expression failures.


58-62: Approve warning log for expression compilation failures; gofmt and go vet pass

@dogancanbakir dogancanbakir merged commit e6c1a80 into dev Oct 14, 2025
21 checks passed
@dogancanbakir dogancanbakir deleted the 6523_make_verbose branch October 14, 2025 15:40
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.

Improve error handling for expression compilation failures in expressions.go

2 participants