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

Exception('Could not retrieve diff. Operation will abort.') #231

Closed
CermakM opened this issue Nov 10, 2024 · 21 comments
Closed

Exception('Could not retrieve diff. Operation will abort.') #231

CermakM opened this issue Nov 10, 2024 · 21 comments

Comments

@CermakM
Copy link

CermakM commented Nov 10, 2024

I am hitting this issue with v5 as well, even tho its explicitly stated the TOKEN is not required and would default to github.token:

Traceback (most recent call last):
  File "/app/main.py", line 931, in <module>
    last_diff = StringIO(client.get_last_diff())
                         ^^^^^^^^^^^^^^^^^^^^^^
  File "/app/main.py", line 112, in get_last_diff
    raise Exception('Could not retrieve diff. Operation will abort.')
Exception: Could not retrieve diff. Operation will abort.

This is my current configuration:

  todo:
    name: Create Issues for code comments
    runs-on: ubuntu-latest
    permissions: read-all
    steps:
      - uses: actions/checkout@v4
      - name: TODO to Issue
        uses: alstr/todo-to-issue-action@v5
        with:
          TOKEN: ${{ github.token }}
          IDENTIFIERS: '[ { "name": "TODO", "labels": [ "todo" ] }, { "name": "FIXME", labels: [ "fixme" ] } ]'
          CLOSE_ISSUES: 'true'
          INSERT_ISSUE_URLS: 'true'
      - name: Set Git user
        run: |
          git config --global user.name  "github-actions[bot]"
          git config --global user.email "github-actions[bot]@users.noreply.github.com"
      - name: Commit and Push issue URLS
        run: |
          git add -A
          if [[ `git status --porcelain` ]]; then
            git commit -m "Automatically added GitHub issue links to TODOs"
            git push origin main
          else
            echo "No changes to commit"
          fi

I tried adding also permissions field but to no affect. My settings are set correctly to allow READ and WRITE. Any idea what' wrong?

@cmathewsendsight
Copy link

Getting the same issue, config is:

name: Create Issue from TODO

on:
    push:
        branches:
            - main
    pull_request:
    workflow_dispatch:
        inputs:
            MANUAL_COMMIT_REF:
                description: 'SHA of the commit to get the diff for'
                required: true
            MANUAL_BASE_REF:
                description: 'Optional: Earlier SHA for comparison'
                required: false

permissions:
    contents: write
    issues: write
    pull-requests: read

jobs:
    create_issue:
        runs-on: ubuntu-latest
        steps:
            - name: Checkout repository
              uses: actions/checkout@v3
              with:
                  fetch-depth: 0

            - name: Run TODO to Issue Action
              uses: alstr/[email protected]
              with:
                  TOKEN: ${{ secrets.GITHUB_TOKEN }}
                  CLOSE_ISSUES: false

          

Based on logs, repo checks out fine. This is a private repo, but token should be working for access, as it does clone down just fine. I did try it on a separate public repo, and it worked without issue, the exact same format, so I suspect its related to privacy somehow but I'm not sure how.

@alstr
Copy link
Owner

alstr commented Nov 22, 2024

@CermakM @cmathewsendsight Please could you try removing TOKEN from your workflow file? The action should have access to the GitHub-supplied token by default, and if you override that with an incorrectly scoped token that may prevent it from working.

@cmathewsendsight
Copy link

I will try that this morning and report back :)

@cmathewsendsight
Copy link

Same result

@alstr
Copy link
Owner

alstr commented Nov 22, 2024

In that case I'm not 100% sure. Do you own the private repo? I've tested it on a private repo and it should work with them, but there might be some permissions issues. You shouldn't need to specify permissions in the workflow file for the action to run, so that might be affecting things. I'd also use @v5 to target the latest v5 release. You could try copying the workflow file from this repository (changing @master to @v5):

name: "Run TODO to Issue"
on:
  push:
  workflow_dispatch:
    inputs:
      MANUAL_COMMIT_REF:
        description: "The SHA of the commit to get the diff for"
        required: true
      MANUAL_BASE_REF:
        description: "By default, the commit entered above is compared to the one directly before it; to go back further, enter an earlier SHA here"
        required: false
jobs:
  build:
    runs-on: "ubuntu-latest"
    steps:
      - uses: "actions/checkout@v4"
      - name: "TODO to Issue"
        uses: "alstr/todo-to-issue-action@master"
        env:
          MANUAL_COMMIT_REF:
            ${{ inputs.MANUAL_COMMIT_REF }}
          MANUAL_BASE_REF:
            ${{ inputs.MANUAL_BASE_REF }}

If none of that works, try creating a classic Personal Access Token with the repo scope. Make sure you enter it in Settings -> Secrets and variables (Actions) -> Secrets, then reference it by the same name in the workflow file: SECRET: "${{ secrets.THE_NAME_OF_YOUR_SECRET }}"

@cmathewsendsight
Copy link

Created a new branch, and new PR. This is my action now:

name: TODO to Issue revised

on:
    push:
    pull_request:
    workflow_dispatch:
        inputs:
            MANUAL_COMMIT_REF:
                description: 'SHA of the commit to get the diff for'
                required: true
            MANUAL_BASE_REF:
                description: 'Optional: Earlier SHA for comparison'
                required: false

jobs:
    create_issue:
        runs-on: ubuntu-latest
        steps:
            - uses: actions/checkout@v4
              with:
                  fetch-depth: 0

            - name: Run TODO to Issue Action
              uses: alstr/todo-to-issue-action@v5
              with:
                  CLOSE_ISSUES: false

It succeeds when running against the commit (note, I've removed the branches-main check, so now it runs on each commit), but not when running against the PR. The diff/diff URL generated is identical between both runs. I don't see any differences actually in the way either of them are run from the action log, aside from the failure to generate diff message. I think this is probably fine, in that I can just merge and it will probably run properly once its in main, but I am still not really sure why it would run against the PR successfully in my test against a public repo, but not against the PR in the private repo. Hopefully that makes sense.

@alstr
Copy link
Owner

alstr commented Nov 26, 2024

Thanks for the info. I just noticed that the action was targeting an old image, and that may have caused a few issues. It might not resolve your problem, but could you try anyway to rule that out? You just need to use the latest version. 😄 If that doesn’t help I’ll look into it further. Thanks.

@maxzitron
Copy link

I'm on the latest version and having the problem running the Action from PR in a Private Repo as well with the same error.

Traceback (most recent call last):
  File "/app/main.py", line 162, in <module>
    last_diff = client.get_last_diff()
                ^^^^^^^^^^^^^^^^^^^^^^
  File "/app/GitHubClient.py", line 90, in get_last_diff
    raise Exception('Could not retrieve diff. Operation will abort.')
Exception: Could not retrieve diff. Operation will abort.

Though, my action file is even more simple.

on:
  - pull_request

jobs:
  sync-todo-to-issue:
    name: 'Sync TODO to Issue'
    runs-on: 'ubuntu-latest'

    steps:
      - name: 🛑 Cancel Previous Runs
        uses: styfle/[email protected]

      - name: ⬇️ Checkout repo
        uses: actions/checkout@v4
        with:
          fetch-depth: 0

      - name: 'TODO to Issue'
        uses: 'alstr/todo-to-issue-action@v5'

Hope this provides @alstr a bit more context.

@dharmendrasha
Copy link

dharmendrasha commented Dec 4, 2024

same issue here

name: Check for TODO comments

on:
  workflow_dispatch:
  pull_request:

jobs:
  check_todos:
    runs-on: ubuntu-latest
    permissions:
      contents: read
      packages: write
      id-token: write
      issues: write

    steps:
      - name: Checkout code
        uses: actions/checkout@v3
        with:
          lfs: true
          fetch-depth: 0

      - name: 'TODO to Issue'
        uses: 'alstr/[email protected]'
        with:
          INSERT_ISSUE_URLS: 'true'
      - name: Set Git user
        run: |
          git config --global user.name "github-actions[bot]"
          git config --global user.email "github-actions[bot]@users.noreply.github.com"
      - name: Commit and Push Changes
        run: |
          git add -A
          if [[ `git status --porcelain` ]]; then
            git commit -m "Automatically added GitHub issue links to TODOs"
            git push origin main
          else
            echo "No changes to commit"
          fi

@alstr
Copy link
Owner

alstr commented Dec 5, 2024

I've just created a test private repo with nothing other than the standard workflow file and a simple Python file and it worked fine for me. That would lead me to think it may be permissions related, but more than likely if the common thread here is the pull_request event, some kind of issue there.

The diff can fail to be retrieved if the commit references it is trying to compare are invalid (e.g., one doesn't exist). This can happen in certain circumstances. I'll shamelessly copy in @rgalonso in case he has any thoughts.

@rgalonso
Copy link
Contributor

rgalonso commented Dec 5, 2024

I don't have a strong reason for saying so, but I suspect that the before value here is not valid.

Making the exception's message more verbose will probably help us run this to ground. The following change should help us resolve both this issue and any related issues in the future:

-        raise Exception('Could not retrieve diff. Operation will abort.')
+        error_response = [f"Could not retrieve diff",
+                          f'URL: {diff_url}',
+                          f'Status code: {diff_request.status_code}']
+        if 'application/json' in diff_request.headers['content-type']:
+            error_response.append(f"Server response: {json.loads(diff_request.text)['message']}")
+        error_response.append('Operation will abort')
+        raise Exception('\n'.join(error_response))

@alstr
Copy link
Owner

alstr commented Dec 6, 2024

Sounds like a plan. I've released a new update with the updated error logging. I actually noticed it when I was in the process of updating the action:

https://github.com/alstr/todo-to-issue-action/actions/runs/12204165092/job/34048676326

URL: https://api.github.com/repos/alstr/todo-to-issue-action/compare/9d7ebbf119260f989e383a111438996cb502472a...f357c404fe00f5a17cd96696eaf6f8ce5fd19cf7
Status code: 404
Server response: Not Found

The problem here being the SHA 9d7ebbf119260f989e383a111438996cb502472a doesn't seem to exist. I'm not too sure why as I haven't noticed this before, though I did force push an update to the v5 tag, which may or may not be related (again, hasn't happened before when doing the same). I'll do a bit more digging when I get chance.

@rgalonso
Copy link
Contributor

rgalonso commented Dec 6, 2024

though I did force push

Yes, that's exactly what I was wondering might be the case. The "before" SHA may no longer exist if a force push removed it from the history.

Those of you seeing this issue: did anyone of you force push the commit in question?

@alstr
Copy link
Owner

alstr commented Dec 9, 2024

I can't remember the exact sequence of events, but yes it might be the force push that caused it in my case.

If this exception occurs - which might be hard to anticipate if it is due to a force push and a nonexistent SHA - we could fall back to trying this endpoint instead:

/commits/f357c404fe00f5a17cd96696eaf6f8ce5fd19cf7

That would retrieve the diff for the last commit, rather than trying to compare two specific commits, but it might be a viable option.

@rgalonso
Copy link
Contributor

rgalonso commented Dec 9, 2024

You might want to make sure GitHub hasn't changed the API implementation. I can't seem to find it now, but the other day when was looking at this I read some doc which said the before field is set to null when not applicable. The code is currently checking for a string of 0s, so it may erroneously still be falling into that if block and producing an invalid URL like: https://api.github.com/repos/alstr/todo-to-issue-action/compare/null...f357c404fe00f5a17cd96696eaf6f8ce5fd19cf7?

@alstr
Copy link
Owner

alstr commented Dec 9, 2024

I've not seen that myself but it's potentially the case. What if we cover all bases with a change like elif self.before not in (None, '0000000000000000000000000000000000000000')? Or is null a string?

@rgalonso
Copy link
Contributor

rgalonso commented Dec 9, 2024

Unclear whether it's a string or not, and how that would end up being represented once having passed through requests and json modules. But, conceptually, I agree that something like not in (None, '0000000000000000000000000000000000000000') (or even not in (None, '0000000000000000000000000000000000000000', 'null') would be helpful.

That said, given your earlier comment, I'm not convinced that's good enough. Perhaps some logic needs to be added based on the forced field here? i.e. if before is invalid OR forced is true, move on to the next block in get_last_diff. It's also worth noting that the link shows there's a compare field. Seems like that would be a more efficient way to handle this, though you may still need to factor forced into the logic.

@alstr alstr closed this as completed in 4547157 Dec 12, 2024
@alstr
Copy link
Owner

alstr commented Dec 12, 2024

Okay, I figured out the problem.

When the action is triggered by a PR, this supplies the diff URL automatically in the form https://github.com/alstr/todo-to-issue-action-test/pull/2.diff.

If you paste this in your browser, it triggers a redirect to https://patch-diff.githubusercontent.com/raw/alstr/todo-to-issue-action-test/pull/2.diff?token=mytoken.

When the action tries to request this URL though, it returns a 404. What I've done is extract the PR number from the supplied URL and rebuild the diff URL utilising the pulls endpoint from the GitHub API, like so: diff_url = f'{self.repos_url}{self.repo}/pulls/{pr_number}'

That then allows it to retrieve the diff in question. I've tested it and it should work. Obviously if you have multiple workflow triggers, you may want to refine that so you're not getting duplicate issues created.

@rgalonso
Copy link
Contributor

Good find. Hopefully that resolves the issue for everyone who was reporting this.

Obviously if you have multiple workflow triggers, you may want to refine that so you're not getting duplicate issues created.

That's a pretty important note. I don't know if that point is already covered by the README, but maybe it's worth saying there.

@alstr
Copy link
Owner

alstr commented Dec 12, 2024

Yeah, I've added it in a couple of places to make it clearer.

I still notice the error when force pushing, so it might be worth adding the fallback to the /commits/ endpoint if /compare/ fails because of an invalid SHA.

@alstr
Copy link
Owner

alstr commented Dec 12, 2024

(I've added the fallback URL and it functioned as expected here after a force push: https://github.com/alstr/todo-to-issue-action/actions/runs/12300435860/job/34328590758)

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

No branches or pull requests

6 participants