Skip to content

feat: enhance error logging with detailed HTML formatting and user/ch…#53

Merged
xbuddhi merged 1 commit into
mainfrom
dev
Oct 14, 2025
Merged

feat: enhance error logging with detailed HTML formatting and user/ch…#53
xbuddhi merged 1 commit into
mainfrom
dev

Conversation

@xbuddhi

@xbuddhi xbuddhi commented Oct 14, 2025

Copy link
Copy Markdown

User description

…at context


PR Type

Enhancement


Description

  • Enhanced error logging with HTML formatting and expandable blockquotes

  • Added detailed user and chat context extraction (username, ID, chat title)

  • Implemented stack trace location tracking with caller information

  • Replaced plain text formatting with HTML-escaped rich formatting


Diagram Walkthrough

flowchart LR
  A["Error Detection"] --> B["Extract User/Chat Context"]
  B --> C["Build HTML Message"]
  C --> D["Add Stack Trace Location"]
  D --> E["Send to Telegram"]
Loading

File Walkthrough

Relevant files
Enhancement
error_logger.go
Enhanced error logging with HTML formatting and context   

internal/telegram/error_logger.go

  • Replaced plain text error formatting with HTML-based formatting using
    utils.EscapeHTML
  • Added extraction of user details (username, first/last name, ID) and
    chat details (title, ID) from userInfo parameters
  • Implemented runtime stack trace capture with runtime.Caller() to
    include file location and function name
  • Wrapped error details in expandable HTML
    with emoji indicators for
    better readability
+51/-4   

Summary by CodeRabbit

  • Chores
    • Enhanced error notifications sent to Telegram with HTML formatting for improved readability.
    • Alerts now include relevant user/chat context, error details, source location, and timestamps.
    • Improved accuracy of source location detection for clearer diagnostics.
    • Notifications are dispatched asynchronously to avoid blocking normal operations.

@qodo-code-review

Copy link
Copy Markdown

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
Overexposed file paths

Description: Using runtime.Caller with a fixed skip (1 then optionally 2) may capture the logger's own
frame depending on inlining and call paths, risking incorrect source location disclosure
to external recipients; verify the skip logic or gate detailed paths behind configuration.

error_logger.go [95-108]

Referred Code
msg += "<blockquote expandable>\n"
msg += "❗ Error: " + htmlEscape(err.Error()) + "\n"
// Add stack trace location if available
if pc, file, line, ok := runtime.Caller(1); ok {
	if strings.Contains(file, "error_logger.go") {
		if pc2, file2, line2, ok2 := runtime.Caller(2); ok2 {
			pc = pc2
			file = file2
			line = line2
		}
	}
	funcName := runtime.FuncForPC(pc).Name()
	msg += fmt.Sprintf("📍 Location: %s:%d in %s\n", htmlEscape(file), line, htmlEscape(funcName))
}
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
No custom compliance provided

Follow the guide to enable custom compliance check.

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@xbuddhi xbuddhi merged commit 8ffa8a4 into main Oct 14, 2025
1 check passed
@coderabbitai

coderabbitai Bot commented Oct 14, 2025

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

Walkthrough

Reworks telegram error logging to build and send HTML-formatted error messages. Extracts user/chat context from provided userInfo, selects a relevant stack frame (excluding error_logger.go when possible), timestamps the message, and dispatches it via a new HTML-specific sender asynchronously.

Changes

Cohort / File(s) Summary of Changes
Telegram error logging: HTML formatting and async send
internal/telegram/error_logger.go
Replaced plaintext/Markdown formatting with HTML message construction; added user/chat context extraction; implemented stack frame selection avoiding self-file; included timestamp; switched to async send via sendToTelegramHTML; removed previous formatErrorMessage path and standard markdown sender usage.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Caller
  participant EL as ErrorLogger
  participant TG as Telegram API (HTML)

  Caller->>EL: LogError(err, userInfo, ctx)
  rect rgba(230,240,255,0.5)
    note right of EL: Extract user/chat from userInfo<br/>Find nearest non-self stack frame<br/>Build HTML message + timestamp
  end
  par Async dispatch
    EL-->>TG: sendToTelegramHTML(htmlMessage)
  end
  TG-->>EL: ack/response (async)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

I tap my paws and format bright,
Errors gleam in HTML light.
With chats and users neatly told,
A timestamp marks the tale so bold.
I ship it off, async flight—
A bunny courier in the night. 🐇✨

✨ 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 dev

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1bd1be9 and afc9c74.

📒 Files selected for processing (1)
  • internal/telegram/error_logger.go (1 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@qodo-code-review

Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Capture full stack traces for errors

Replace runtime.Caller, which captures a single stack frame, with runtime.Stack
to capture the complete stack trace for more comprehensive error debugging.

Examples:

internal/telegram/error_logger.go [98-108]
	if pc, file, line, ok := runtime.Caller(1); ok {
		if strings.Contains(file, "error_logger.go") {
			if pc2, file2, line2, ok2 := runtime.Caller(2); ok2 {
				pc = pc2
				file = file2
				line = line2
			}
		}
		funcName := runtime.FuncForPC(pc).Name()
		msg += fmt.Sprintf("📍 Location: %s:%d in %s\n", htmlEscape(file), line, htmlEscape(funcName))

 ... (clipped 1 lines)

Solution Walkthrough:

Before:

func (el *ErrorLogger) LogError(err error, context string, userInfo ...interface{}) {
    // ...
    msg := "..."
    msg += "<blockquote expandable>\n"
    msg += "❗ Error: " + htmlEscape(err.Error()) + "\n"
    // Add stack trace location if available
    if pc, file, line, ok := runtime.Caller(1); ok {
        // ... logic to skip one stack frame
        funcName := runtime.FuncForPC(pc).Name()
        msg += fmt.Sprintf("📍 Location: %s:%d in %s\n", htmlEscape(file), line, htmlEscape(funcName))
    }
    msg += "🕒 Time: " + htmlEscape(timestamp) + "\n"
    msg += "</blockquote>"
    // ...
}

After:

func (el *ErrorLogger) LogError(err error, context string, userInfo ...interface{}) {
    // ...
    buf := make([]byte, 4096) // Create a buffer for the stack trace
    n := runtime.Stack(buf, false)
    stackTrace := string(buf[:n])

    msg := "..."
    msg += "<blockquote expandable>\n"
    msg += "❗ Error: " + htmlEscape(err.Error()) + "\n"
    msg += "🥞 Stack Trace:\n<pre><code>" + htmlEscape(stackTrace) + "</code></pre>\n"
    msg += "🕒 Time: " + htmlEscape(timestamp) + "\n"
    msg += "</blockquote>"
    // ...
}
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies that using runtime.Caller provides limited debugging context and proposes using runtime.Stack for a full trace, which is a significant and valuable enhancement to the error logging feature being introduced.

Medium
Possible issue
Prevent panic from nil pointer

Prevent a potential panic by checking if runtime.FuncForPC(pc) returns nil
before calling the .Name() method, and use a fallback value if it does.

internal/telegram/error_logger.go [106]

-funcName := runtime.FuncForPC(pc).Name()
+fn := runtime.FuncForPC(pc)
+funcName := "unknown function"
+if fn != nil {
+    funcName = fn.Name()
+}
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a potential nil pointer dereference that could cause the error logger to panic, and provides a robust fix to prevent the crash, which is a critical improvement for stability.

Medium
Improve stack trace location logic

Improve the stack trace logic by iterating up the call stack to find the first
caller outside of error_logger.go, ensuring the correct error location is
reported.

internal/telegram/error_logger.go [98-108]

-if pc, file, line, ok := runtime.Caller(1); ok {
-    if strings.Contains(file, "error_logger.go") {
-        if pc2, file2, line2, ok2 := runtime.Caller(2); ok2 {
-            pc = pc2
-            file = file2
-            line = line2
-        }
-    }
+// Add stack trace location if available
+pc, file, line, ok := runtime.Caller(1)
+for i := 2; ok && strings.Contains(file, "error_logger.go"); i++ {
+    pc, file, line, ok = runtime.Caller(i)
+}
+if ok {
     funcName := runtime.FuncForPC(pc).Name()
     msg += fmt.Sprintf("📍 Location: %s:%d in %s\n", htmlEscape(file), line, htmlEscape(funcName))
 }
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly points out a flaw in the stack trace logic and proposes a more robust loop-based solution to find the true origin of the error, significantly improving the usefulness of the error log.

Medium
General
Simplify conditional for user details

Simplify the conditional for adding user details to the error message by
removing the redundant userStr != "" check.

internal/telegram/error_logger.go [89-91]

-if userStr != "" && userId != "" {
+if userId != "" {
     msg += "👤 User: " + userStr + " (ID: " + userId + ")\n"
 }
  • Apply / Chat
Suggestion importance[1-10]: 3

__

Why: The suggestion correctly identifies a redundant check, as userStr is initialized to "Unknown" and is never empty, making the check on userId sufficient. This is a minor code quality improvement.

Low
  • More

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants