Athens: Go get returns 500 after the repository is deleted

[arschles]

I created github.com/arschles/throwaway1, did a go get on it with GOPROXY set to https://microsoftgoproxy.azurewebsites.net, and then deleted the repository. The next go get returns this:

arschles ~/Desktop/hellogc $ GO111MODULE=on GOPROXY=https://microsoftgoproxy.azurewebsites.net go get github.com/arschles/throwaway1@v1
go get github.com/arschles/throwaway1@v1: unexpected status (https://microsoftgoproxy.azurewebsites.net/github.com/arschles/throwaway1/@v/list): 500 Internal Server Error

I think this violates immutable builds? What do folks think?

[marpio]

Makes sense. We use go to get the list of versions. We should probably merge it with what we have in storage.

[arschles]

@marpio which do you think we should check first - go or storage?

[marpio]

Couldn't we just return a merger list from the /list endpoint?
So if storage gives us
[email protected]
[email protected]
[email protected]
and go list gives us
[email protected]
[email protected]
[email protected]
Then the proxy /list endpoint would return
[email protected]
[email protected]
[email protected]
[email protected]
What do you think @arschles ?

[arschles]

I agree, that sounds good to me. If go list fails, would we just returns what storage gives?

[marpio]

Yes, and if storage finds nothing we would just propagate the go error since the module didn't exist (or possibly was deleted from VCS before proxy ever saw it)

[arschles]

That sounds like a good plan to me. @marwan91 what do you think? I remember
you implemented that upstream go list logic?

On Thu, Aug 30, 2018 at 11:28 marpio [e-mail] wrote:

Yes, and if storage finds nothing we would just propagate the go error
since the module doesn't exist (or possibly was deleted from VCS before
proxy ever saw it)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/gomods/athens/issues/590#issuecomment-417401447, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAEU0TkEmsgXb6CKHcaV2CI8zwc9JwGnks5uWCDAgaJpZM4WT0zn
.

[robjloranger]

That should definitely not be happening, Olympus should have that stored.

[marpio]

@robjloranger I think the proxy has it in it's storage but go list -version which we use under the hood checks only the VCS.

[robjloranger]

I like your merge idea @marpio, that way we at least give what we have when vcs fails

[marpio]

I would like to work on that if that's fine with everyone.

[marwan-at-work]

We're starting to hit a few of the downsides of using the go binary in the background: can we gracefully handle errors such as "repository deleted" vs "unexpected error". Meaning, if a repo is deleted, we should gracefully ignore that error. But if there's a true error of some kind, we should probably error out.

The go binary doesn't return structured errors, so we're gonna have to parse the text. Here's what the go command returns when a repository is deleted:

$ go list -m -versions -json github.com/pkg/fakerepo@latest
go list -m github.com/pkg/fakerepo: git ls-remote -q https://github.com/pkg/fakerepo in /Users/me/go/pkg/mod/cache/vcs/f515d433f0460174c81da03b4c26d60d540854bb3dfad2fbf0798881fe72b6c7: exit status 128:
    remote: Repository not found.
    fatal: repository 'https://github.com/pkg/fakerepo/' not found

[arschles]

@marwan-at-work do you think it's time we ask for structured output from the Go CLI?

[marwan-at-work]

@arschles yes that'd be nice.