Skip to content
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

Posting comment(s) failed at the second run #46

Closed
shenxianpeng opened this issue Apr 3, 2022 · 8 comments
Closed

Posting comment(s) failed at the second run #46

shenxianpeng opened this issue Apr 3, 2022 · 8 comments
Labels
bug Something isn't working

Comments

@shenxianpeng
Copy link
Collaborator

shenxianpeng commented Apr 3, 2022

post push comments failed when comments exist

Posting comment(s)
  INFO:CPP Linter:comments_url: https://api.github.com/repos/shenxianpeng/cpp-linter-action/commits/f4c83bab3681622d96b63bc60abeec966bdf4968/comments
  Traceback (most recent call last):
    File "/usr/lib/python3.8/runpy.py", line 194, in _run_module_as_main
      return _run_code(code, main_globals, None,
    File "/usr/lib/python3.8/runpy.py", line 87, in _run_code
      exec(code, run_globals)
    File "/github/workspace/cpp_linter/run.py", line 811, in <module>
      main()
    File "/github/workspace/cpp_linter/run.py", line 805, in main
      post_results(False)  # False is hard-coded to disable diff comments.
    File "/github/workspace/cpp_linter/run.py", line [67](https://github.com/shenxianpeng/cpp-linter-action/runs/5806368569?check_suite_focus=true#step:4:67)9, in post_results
      checks_passed = post_push_comment(base_url, user_id)
    File "/github/workspace/cpp_linter/run.py", line 541, in post_push_comment
      remove_bot_comments(comments_url, user_id)
    File "/github/workspace/cpp_linter/thread_comments.py", line 28, in remove_bot_comments
      and comment["body"].startswith("<!-- cpp linter action -->")
  KeyError: 'body'

More details https://github.com/shenxianpeng/cpp-linter-action/runs/5806368569?check_suite_focus=true

@shenxianpeng shenxianpeng added the bug Something isn't working label Apr 3, 2022
@2bndy5
Copy link
Collaborator

2bndy5 commented Apr 3, 2022

Getting this error on a public repo makes me think github has changed how a thread's comments are returned. If they switch everything from using body to body_text field, then we may not be able to continue using an HTML comment to identify a bot's comment from other actions' bot comments.

Personally, I find the bot comments to be annoying, so I always disable them.

@shenxianpeng
Copy link
Collaborator Author

shenxianpeng commented Apr 5, 2022

See from the comments_url: https://api.github.com/repos/shenxianpeng/cpp-linter-action/commits/f4c83bab3681622d96b63bc60abeec966bdf4968/comments, it seems still using body

Personally, I find the bot comments to be annoying, so I always disable them.

So we could change thread-comments default value to false. your thoughts?

@2bndy5
Copy link
Collaborator

2bndy5 commented Apr 5, 2022

Changing the default value will only abate the issue. I'm not sure why the error is happening.

This is what I saw when I looked into the error on the test private repo
[
    {
        "url": "https://api.github.com/repos/shenxianpeng/test-cpp-linter-action/issues/comments/1074807551",
        "html_url": "https://github.com/shenxianpeng/test-cpp-linter-action/pull/10#issuecomment-1074807551",
        "issue_url": "https://api.github.com/repos/shenxianpeng/test-cpp-linter-action/issues/10",
        "id": 1074807551,
        "node_id": "IC_kwDOFY2uzM5AEEL_",
        "user": {
            "login": "github-actions[bot]",
            "id": 41898282,
            "node_id": "MDM6Qm90NDE4OTgyODI=",
            "avatar_url": "https://avatars.githubusercontent.com/in/15368?v=4",
            "gravatar_id": "",
            "url": "https://api.github.com/users/github-actions%5Bbot%5D",
            "html_url": "https://github.com/apps/github-actions",
            "followers_url": "https://api.github.com/users/github-actions%5Bbot%5D/followers",
            "following_url": "https://api.github.com/users/github-actions%5Bbot%5D/following{/other_user}",
            "gists_url": "https://api.github.com/users/github-actions%5Bbot%5D/gists{/gist_id}",
            "starred_url": "https://api.github.com/users/github-actions%5Bbot%5D/starred{/owner}{/repo}",
            "subscriptions_url": "https://api.github.com/users/github-actions%5Bbot%5D/subscriptions",
            "organizations_url": "https://api.github.com/users/github-actions%5Bbot%5D/orgs",
            "repos_url": "https://api.github.com/users/github-actions%5Bbot%5D/repos",
            "events_url": "https://api.github.com/users/github-actions%5Bbot%5D/events{/privacy}",
            "received_events_url": "https://api.github.com/users/github-actions%5Bbot%5D/received_events",
            "type": "Bot",
            "site_admin": false
        },
        "created_at": "2022-03-22T07:04:40Z",
        "updated_at": "2022-03-22T07:04:40Z",
        "author_association": "NONE",
        "body_text": "\ud83d\udcdc Run clang-format on the following files\n\n demo.hpp\n demo.cpp\n\n\n\ud83d\udcac Output from clang-tidy\ndemo.hpp\n\ndemo.hpp:10:11: warning: [modernize-use-trailing-return-type]\n\nuse a trailing return type for this function\n\n\n    void *not_usefull(char *str){\n    ~~~~~~^\n    auto                         -> void *\n\n\n\ndemo.hpp:12:16: warning: [modernize-use-nullptr]\n\nuse nullptr\n\n\n        return 0;\n               ^\n               nullptr\n\n\ndemo.cpp\n\ndemo.cpp:5:5: warning: [modernize-use-trailing-return-type]\n\nuse a trailing return type for this function\n\n\nint main()\n~~~ ^\nauto       -> int\n\n\n\ndemo.cpp:7:13: warning: [readability-braces-around-statements]\n\nstatement should be inside braces\n\n\n    for (;;)\n            ^\n             {\n\n\n\ndemo.cpp:10:5: warning: [cppcoreguidelines-pro-type-vararg]\n\ndo not call c-style vararg functions\n\n\n    printf(\"Hello world!\\n\");\n    ^",
        "reactions": {
            "url": "https://api.github.com/repos/shenxianpeng/test-cpp-linter-action/issues/comments/1074807551/reactions",
            "total_count": 0,
            "+1": 0,
            "-1": 0,
            "laugh": 0,
            "hooray": 0,
            "confused": 0,
            "heart": 0,
            "rocket": 0,
            "eyes": 0
        },
        "performed_via_github_app": null
    },
]

@shenxianpeng
Copy link
Collaborator Author

OK, got your point. The current bug is weird and cannot be reproduced using comments_url https://api.github.com/repos/shenxianpeng/cpp-linter-action/commits/f4c83bab3681622d96b63bc60abeec966bdf4968/comments

@2bndy5
Copy link
Collaborator

2bndy5 commented Apr 5, 2022

The comments_url is slightly different for PR vs push events. If I had to guess, I'd say the remove_bot_comments() is iterating over the fetched thread comments incorrectly??

@shenxianpeng
Copy link
Collaborator Author

shenxianpeng commented Apr 5, 2022

Tested again and found that this issue was caused by headers=API_HEADERS. So the public repository might don't need to pass in headers.

When headers=API_HEADERS exist, reproduce the error

import requests
from requests import Response

class Globals:
    response_buffer = Response()


API_HEADERS = {"Accept": "application/vnd.github.v3.text+json",}
# if GITHUB_TOKEN:
#     API_HEADERS["Authorization"] = f"token {GITHUB_TOKEN}"

comments_url = 'https://api.github.com/repos/shenxianpeng/cpp-linter-action/commits/f4c83bab3681622d96b63bc60abeec966bdf4968/comments'
Globals.response_buffer = requests.get(comments_url, headers=API_HEADERS)

comments = Globals.response_buffer.json()
for comment in comments:
    if (comment["body"].startswith("<!-- cpp linter action -->")): 
        print("Success")
$ python3 test.py 
Traceback (most recent call last):
  File "test.py", line 17, in <module>
    if (comment["body"].startswith("<!-- cpp linter action -->")): 
KeyError: 'body'

When headers=API_HEADERS not exist, run again, success.

import requests
from requests import Response

class Globals:
    response_buffer = Response()


API_HEADERS = {"Accept": "application/vnd.github.v3.text+json",}
# if GITHUB_TOKEN:
#     API_HEADERS["Authorization"] = f"token {GITHUB_TOKEN}"

comments_url = 'https://api.github.com/repos/shenxianpeng/cpp-linter-action/commits/f4c83bab3681622d96b63bc60abeec966bdf4968/comments'
Globals.response_buffer = requests.get(comments_url)

comments = Globals.response_buffer.json()
for comment in comments:
    if (comment["body"].startswith("<!-- cpp linter action -->")): 
        print("Success")
$ python3 test.py 
Success

@2bndy5
Copy link
Collaborator

2bndy5 commented Apr 5, 2022

If that's the case, then it should be an easy fix.

2bndy5 added a commit that referenced this issue Apr 5, 2022
This should resolve #46. It will not affect the fix for private repos since thread comments are now auto-disabled for private repos.
@2bndy5
Copy link
Collaborator

2bndy5 commented Apr 5, 2022

ok I pushed the fix for unnecessarily using a token when fetching the thread comments on the update-thread-comments branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants