Skip to content

Logs an error with code extraction and details #11931

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

JaynieBai
Copy link
Member

@JaynieBai JaynieBai commented May 30, 2025

Fixes #8785

Context

Re logging, the XslTransformation task calls TaskLoggingHelper.LogErrorWithCodeFromResources(string messageResourceName, params object[] messageArgs). There is also TaskLoggingHelper.LogErrorFromException(Exception exception, bool showStackTrace, bool showDetail, string file), which logs the inner exceptions if requested via the showDetail parameter or the "MSBUILDDIAGNOSTICS" environment variable, but this then does not log an error code

Changes Made

Add a new log method logErrorWithCodeAndException which Logs an error using a resource string (with code extraction and help keyword) and appends exception details.

Testing

Notes

@JaynieBai JaynieBai self-assigned this Jun 4, 2025
@JaynieBai JaynieBai marked this pull request as ready for review June 4, 2025 07:55
@Copilot Copilot AI review requested due to automatic review settings June 4, 2025 07:55
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds a new logging method to include both resource-based error codes and detailed exception information, and replaces separate error and message calls in the XslTransformation task with this combined call.

  • XslTransformation now uses LogErrorWithCodeAndException instead of two separate logging calls
  • TaskLoggingHelper introduces LogErrorWithCodeAndException and extracts duplicated exception formatting into GetFormattedExceptionDetails

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/Tasks/XslTransformation.cs Swapped individual error and message logs for a single call to LogErrorWithCodeAndException
src/Shared/TaskLoggingHelper.cs Added LogErrorWithCodeAndException, refactored exception formatting into helper
Comments suppressed due to low confidence (2)

src/Shared/TaskLoggingHelper.cs:1533

  • [nitpick] The parameter order (file, then messageResourceName) differs from LogErrorWithCodeFromResources. To avoid confusion, consider reordering parameters for consistency across logging methods.
public void LogErrorWithCodeAndException(

src/Shared/TaskLoggingHelper.cs:1539

  • The new method LogErrorWithCodeAndException and GetFormattedExceptionDetails lack direct unit tests. Please add tests to verify correct extraction of error codes, help keywords, and the formatting of exception details under different showStackTrace/showDetail settings.
params object[] messageArgs

string flattenedMessage = TaskLoggingHelper.GetInnerExceptionMessageString(e);
Log.LogErrorWithCodeFromResources("XslTransform.TransformError", flattenedMessage);
Log.LogMessage(MessageImportance.Low, e.ToString());
Log.LogErrorWithCodeAndException(e, true, true, null, "XslTransform.TransformError", e.Message);
Copy link
Preview

Copilot AI Jun 4, 2025

Choose a reason for hiding this comment

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

Passing e.Message as the sole messageArgs may omit inner exception details compared to the previous GetInnerExceptionMessageString. Consider pre-flattening the exception (e.g., via GetFormattedExceptionDetails or its own helper) so the resource formatting includes full context.

Suggested change
Log.LogErrorWithCodeAndException(e, true, true, null, "XslTransform.TransformError", e.Message);
Log.LogErrorWithCodeAndException(e, true, true, null, "XslTransform.TransformError", GetFormattedExceptionDetails(e));

Copilot uses AI. Check for mistakes.

@JaynieBai JaynieBai marked this pull request as draft June 4, 2025 07:59
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.

TaskLoggingHelper logging improvement with error code and inner exceptions.
1 participant