[go: up one dir, main page]
Skip to content

Improve NoteDiffFileCreationError error message to include merge request ID for easier debugging

Summary

The NoteDiffFileCreationError exception currently provides limited context for debugging, making it difficult to identify which merge request is affected when the error occurs during imports or other operations. The error message only includes file path and line information but lacks the merge request ID, which is crucial for debugging and troubleshooting.

Steps to reproduce

  1. Perform a Direct Transfer import of a project with merge requests containing diff notes
  2. Encounter a NoteDiffFileCreationError during the import process
  3. Observe that the error message only shows: "Failed to find diff line for: <file_path>, old_line: <old_line>, new_line: <new_line>"

Current behavior

The NoteDiffFileCreationError exception message format is:

Failed to find diff line for: <file_path>, old_line: <old_line>, new_line: <new_line>

Example from logs:

exception.message: "Failed to find diff line for: src/example/file.c, old_line: , new_line: 945"

Expected behavior

The error message should include the merge request ID to facilitate debugging:

Failed to find diff line for: <file_path>, old_line: <old_line>, new_line: <new_line> (MR: <merge_request_iid>)

This would help administrators and support teams quickly identify which merge request is affected and take appropriate action.

Relevant logs and/or screenshots

The error occurs in app/models/diff_note.rb:48 in the create_diff_file method:

# Current implementation raises error without MR context
raise NoteDiffFileCreationError, "Failed to find diff line for: #{file_path}, old_line: #{old_line}, new_line: #{new_line}"

Environment

  • GitLab version: Affects multiple versions (observed in 17.8.7, 17.10.4)
  • Component: Import/Export, DiffNote model

Possible fixes

The DiffNote model has access to the associated merge request through the noteable association. The error message could be enhanced to include the merge request IID:

mr_info = noteable.respond_to?(:iid) ? " (MR: #{noteable.iid})" : ""
raise NoteDiffFileCreationError, "Failed to find diff line for: #{file_path}, old_line: #{old_line}, new_line: #{new_line}#{mr_info}"

Additional context

This improvement would significantly help with:

  • Debugging import failures
  • Identifying problematic merge requests during bulk operations
  • Providing better error context for support teams
  • Reducing time spent investigating which MR is affected by the error