Create comment fails with error : empty line_code

[aravind2612krishna]

Bug Description

The create comment request returns the following response
{"message":"400 Bad request - Note {:line_code=>[\"can't be blank\", \"must be a valid line code\"]}"}
The request body is as follows:

{
  "body": "```suggestion:-4+0\n                redactedcode\n                    redactedcode-\u003eDestroy();\n\n                if (redactedcode)\n                    redactedcode-\u003eDestroy();\n```\n\nredactedcomment",
  "position": {
    "base_sha": "a2610248747681941279bd033169980fefd25732",
    "head_sha": "d08978d3d18fdfbd9945952d58c50507dc6a23ed",
    "start_sha": "a2610248747681941279bd033169980fefd25732",
    "new_path": "redactedfile",
    "old_path": "redactedfile",
    "position_type": "text",
    "new_line": 1659,
    "old_line": 1979,
    "line_range": {
      "start": {
        "line_code": "ecaaaac4a1a5cb79b55c2e7bf498f9e3847f87d0_1975_1655",
        "type": "new"
      },
      "end": {
        "line_code": "ecaaaac4a1a5cb79b55c2e7bf498f9e3847f87d0_1979_1659",
        "type": "new"
      }
    }
  }
}

Reproduction Steps

call to create_comment(), create_multiline_comment() or create_comment_suggestion(),
It is not rare, some times it works.

[johnybx]

Hi,
I would need more information about this because I cannot reproduce it:

• was the end of selection outside of changed lines ( asking because should be when both new_line and old_line were specified )
• when you open gitlab UI and check the region you selected does the numbers match the request ? When you open MR in gitlab UI and point mouse over the line number you should see URL in form /merge_requests/XYZ/diffs#line_code which should match line_code in request.

[harrisoncramer]

@johnybx I've noticed that you can reliably trigger this error in two different ways:

1. If you try to leave a comment on an unchanged line
2. If you try to leave a multi-line comment and the mutli-line comment spans across unchanged lines

[aravind2612krishna]

@johnybx I've noticed that you can reliably trigger this error in two different ways:

1. If you try to leave a comment on an unchanged line

2. If you try to leave a multi-line comment and the mutli-line comment spans across unchanged lines

I can confirm this is the issue. Thanks @harrisoncramer , I was struggling to reproduce this.

[aravind2612krishna]

Might be related to inherent issue with gitlab
https://gitlab.com/gitlab-org/gitlab/-/issues/24328#note_1682317482

[aravind2612krishna]

I'm not sure what is to be done in this plugin since gitlab itself doesn't support it. Closing for now.

[johnybx]

🤔 that is interesting because I was able to create comments outside of changed lines as long as file itself has at least some changes 🤔 for example I have testing repo where I test various things and I can do comments like this

So if I get it correctly the problem occurs only if the file itself is unchanged ?

[aravind2612krishna]

The problem occurred even when file is changed, and even on a changed line. I'm not able to consistently reproduce it.

Now in my efforts to reproduce it, I see another error:

-- REQUEST --
POST //api/v4/projects/38490095/merge_requests/2665/discussions HTTP/1.1
Host: gitlab.com
Accept: application/json
Content-Type: application/json
Private-Token: REDACTED
User-Agent: go-gitlab

{"body":"What is the difference between these two variables?","position":{"base_sha":"b49aac69176897d2e94307de4cf431dd2a60c75e","head_sha":"65b6a3427f8ad53b8e4a3c5b3d7112a042d9c000","start_sha":"b49aac69176897d2e94307de4cf431dd2a60c75e","new_path":"hm/amesh/utils/_private/normalsadjuster.cxx","old_path":"hm/amesh/utils/_private/normalsadjuster.cxx","position_type":"text","new_line":32,"line_range":{"start":{"line_code":"46cba909aa00eb365f44b5a77986b8afa1c827fb_1_31","type":"new"},"end":{"line_code":"46cba909aa00eb365f44b5a77986b8afa1c827fb_1_32","type":"new"}}}}

-- RESPONSE --
HTTP/2.0 308 Permanent Redirect
Content-Length: 0
Cf-Cache-Status: DYNAMIC
Cf-Ray: 834ddc6efe8b3c01-BLR
Date: Wed, 13 Dec 2023 11:28:00 GMT
Location: https://gitlab.com/api/v4/projects/38490095/merge_requests/2665/discussions
Nel: {"success_fraction":0.01,"report_to":"cf-nel","max_age":604800}
Report-To: {"endpoints":[{"url":"https:\/\/a.nel.cloudflare.com\/report\/v3?s=cfKtXc0GsrClFH70gVPfoTPOviIxddG2hOrwbKQ0kazFbhM7W%2FkEMlOSLHYT%2BEmWTrx6KxRZ7s0%2BRUes8uWU7D%2FQdMG6v0o4IWGcFj2cLhiyhvcQ2eIoXuk9QjQ%3D"}],"group":"cf-nel","max_age":604800}
Server: cloudflare
Set-Cookie: _cfuvid=mXHBVYQ.K.E6hIX4D.bXIOcmWbYL.g.b2cXhZN_4R5E-1702466880071-0-604800000; path=/; domain=.gitlab.com; HttpOnly; Secure; SameSite=None
Strict-Transport-Security: max-age=31536000
X-Content-Type-Options: nosniff

[harrisoncramer]

Thanks for filing this issue.

Gitlab's API surrounding line ranges is very particular and difficult to work with. I'm going to merge in an MR that fixes some of these issues but certainly not all of them. I'll leave this MR open because I'm sure there are other situations that I did not address.

I think longer term we should come up with a more robust approach to comments and line ranges, but it might involve me getting in touch with folks at Gitlab to better understand when the API expects certain values so that we can do a holistic refactor of the ranged/single-line commenting code.

[aravind2612krishna]

Thanks for the merge. It still doesn't fix things, however, as I got the same error today after updating.
I'll try to have a look at this when I get a breather, but looks like the old_line that is passed in the request is not always right.

[jakubbortlik]

I've encountered this right now with gitlab.nvim version 1.2.2.

This is a screenshot of the problematic code:

You can see that Git is actually quite confused about what parts have been deleted, changed, and added. I've tried to make a change suggestion on lines 214 and 215 in the new version (right-hand side). This is the error I got:

gitlab.nvim: Could not create discussion: POST https://gitlab.cloud.phonexia.com//api/v4/projects/42/merge_requests/283/discussions: 400 {message: 400 Bad request - Note {:line_code=>["can't be blank", "must be a valid line code"]}}

Then I made the suggestion on Gitlab in the browser and now I'm getting the following error when I try to go to the comment from the discussion panel or from Diffview's file panel:

Error executing vim.schedule lua callback: Vim:E885: Not possible to change sign gitlab_discussion                                                                                                                                                                      
stack traceback:
        [C]: in function 'sign_placelist'
        ...e/nvim/lazy/gitlab.nvim/lua/gitlab/reviewer/diffview.lua:257: in function 'place_sign'
        ...azy/gitlab.nvim/lua/gitlab/actions/discussions/signs.lua:36: in function 'refresh_signs'
        ...lazy/gitlab.nvim/lua/gitlab/actions/discussions/init.lua:62: in function 'callback'
        ...lazy/gitlab.nvim/lua/gitlab/actions/discussions/init.lua:44: in function 'callback'
        ...ub/.local/share/nvim/lazy/gitlab.nvim/lua/gitlab/job.lua:40: in function ''
        vim/_editor.lua: in function ''
        vim/_editor.lua: in function <vim/_editor.lua:0>

Actually, gitlab.nvim shows some indication of the comment - it shows the line range 193-200 in the "old" revision. But I'd like to have it on lines 214 and 215 in the "new" revision.

And this is the relevant part of the gitlab.nvim log if it's any help:

{
    "id": "d0cb027b8807ced2f896a49876e84bbe23621feb",
    "individual_note": false,
    "notes": [
        {
            "id": 91041,
            "type": "DiffNote",
            "body": "It took me a while to understand that there is implicit string concatenation. I find\nthis more readable:\n```suggestion:-1+0\n            'description': (\n                f'Result response of Speech to Text task.\\n\\n{responses.TASK_RESULT_FIELDS_INFO}'\n            ),\n```",
            "attachment": null,
            "author": {"..."},
            "created_at": "2024-01-18T17:42:17.961Z",
            "updated_at": "2024-01-18T17:42:17.961Z",
            "system": false,
            "noteable_id": 7064,
            "noteable_type": "MergeRequest",
            "project_id": 42,
            "commit_id": null,
            "position": {
                "base_sha": "9e772112d56512ab8a9f87a7a8fd85e37cbb73f1",
                "start_sha": "9e772112d56512ab8a9f87a7a8fd85e37cbb73f1",
                "head_sha": "db7559499880de943a0012db1feaa740475c3dac",
                "old_path": "...",
                "new_path": "...",
                "position_type": "text",
                "old_line": null,
                "new_line": 215,
                "line_range": {
                    "start": {
                        "line_code": "b8fac85c33ccd1d48c29728a5d766f4264ff9255_193_190",
                        "type": null,
                        "old_line": 193,
                        "new_line": 190,
                    },
                    "end": {
                        "line_code": "b8fac85c33ccd1d48c29728a5d766f4264ff9255_200_191",
                        "type": "new",
                        "old_line": null,
                        "new_line": 191,
                    },
                },
            },
            "resolvable": true,
            "resolved": false,
            "resolved_by": null,
            "resolved_at": null,
            "confidential": false,
            "internal": false,
            "noteable_iid": 283,
            "commands_changes": {},
        }
    ],
}