Fixmie should not fail on temporary 5xx errors returned by the Github API

[gbbr]

Please check out DataDog/dd-trace-go#438

[welcome]

Thank you for creating the issue! We'll get back to you shortly with additional information.

[fatih]

I just checked the logs on my end and saw this:

couldn't process event [error=couldn't apply comments: failed removing old suggestions: 1 error occurred: * DELETE https://api.github.com/repos/DataDog/dd-trace-go/pulls/comments/284269129: 404 Not Found [] ]

Did you applied the suggestion directly? Was there anything else you did differently? Seems like there is a race condition. I fetched the existing comments, Fixmie saw that one of the comments is invalidated, hence it tried to delete them, but then it tried to delete the PR comment it was gone at that point. Not sure why that was caused.

[gbbr]

Thanks for looking into this @fatih, I asked the contributor to try and answer your question.

FWIW it happened again today, here: DataDog/dd-trace-go#440. This was one of my own PRs and I haven't had any suggestions from the bot, it simply showed failure.

[fatih]

The latest issue is because of this:

"[ERRO] couldn't process event  [error=couldn't download pull request files: 1 error occurred:
	* couldn't download pull request file: GET https://api.github.com/repos/DataDog/dd-trace-go/contents/ddtrace/tracer?ref=02fae9631226d22cbddc68a99f6a0892bf729d13: 401 Bad credentials []

This is the first time I see this though. Fixmie is a GH application, hence it doesn't store any credentials. It creates a client based on the incoming installation and app id, which is verified and signed by GitHub.

[CAFxX]

I just checked the logs on my end and saw this:

couldn't process event [error=couldn't apply comments: failed removing old suggestions: 1 error occurred: * DELETE https://api.github.com/repos/DataDog/dd-trace-go/pulls/comments/284269129: 404 Not Found [] ]

Did you applied the suggestion directly? Was there anything else you did differently? Seems like there is a race condition. I fetched the existing comments, Fixmie saw that one of the comments is invalidated, hence it tried to delete them, but then it tried to delete the PR comment it was gone at that point. Not sure why that was caused.

This was my first PR in this repo, and the first time I heard about fixmie. A few moments after opening the PR the comment from fixmie appeared, including a code suggestion. As the suggestion was trivial (whitespace change) I almost immediately clicked on apply suggestion. I did not do anything else.

[fatih]

I see, thanks @CAFxX for the feedback. I think immediately is the key here. There is a possible race condition on my side where things can happen between fetching comments and applying them. I'm going to look into it, let's keep this up meanwhile.

[gbbr]

Another one happened here: DataDog/dd-trace-go#442

It might go away by the time you check though, as I'm actively committing to this PR.

Hope this helps in some way :-/ Maybe these are cases that are worthy of a retry before erroring out?

[fatih]

I've added couple of improvements and when this happens again, I'm going to have more insight on what's wrong. I know this happened but I don't have any logs unfortunately for these events. Please just let me know if this happens again. Next time, you'll also going to see a code next to the email address, which will help me to see what went wrong.

[gbbr]

Sure. Thanks! I can update this issue if it happens again.

[gbbr]

Here's one more in DataDog/dd-trace-go#462

[fatih]

Thanks Gabriel. Just checked the logs and all I see is this:

POST https://api.github.com/repos/DataDog/dd-trace-go/statuses/024cbbfc5c9e5c89bfa06e72fdb4086d04a9d055: 500

I think there was a hiccup in the GitHub API. In overall most of these failures are due GitHub API timeouts or failures. I'll make sure to handle all these in a better way and only return a failure if it makes sense. I think showing "Failure" as a status is not much useful for the end user.

[gbbr]

Thanks Faith! One suggestion would be to retry 5xx errors up to a specific timeout. I will stop reporting these until I hear updates on the issue. Happy to help test further :) I think Fixmie does saves some review time which is great, so we'll continue using it on this repo. I'll update the issue title too to be more specific, but feel free to edit it to whatever makes more sense to you.

[gbbr]

Got another one here: DataDog/dd-trace-go#472. Would be interesting to add a link to a retry button.

I'm uninstalling it for now, it fails too often, but happy to add it back once this issue is fixed, or at least a "retry" button is added.

[fatih]

@gbbr thanks! Seems like this is a go list issue :/

go [list -e -json -compiled=true -test=true -export=false -deps=true -find=false -- github.com/DataDog/dd-trace-go/ddtrace/tracer]: exit status 1: go tool cgo: exit status 2  fatal error: runtime: out of memory  

I'm trying to reduce the memory footprint, but it's still not at a place where I like it. go/packages is still a new package with needs some love and performance improvements. Thanks again for reporting.

[fatih]

at least a "retry" button is added.

If you force push or add any new comment, fixmie will check the pull request again.