Skip to content

Commit

Permalink
Return 404 error when policy-bot is not installed (#303)
Browse files Browse the repository at this point in the history
Previously, the server returned a 500 error with no details when trying
to view details for a PR where policy-bot was not installed. This was
most likely to happen if people followed existing links in a repository
that was moved between organizations.

Ideally, this would use a nice HTML 404 page, but for now, just include
a more descriptive plain text message.
  • Loading branch information
bluekeyes authored May 17, 2021
1 parent d7bcb2f commit b80414e
Showing 1 changed file with 17 additions and 4 deletions.
21 changes: 17 additions & 4 deletions server/handler/details.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/alexedwards/scs"
"github.com/bluekeyes/templatetree"
"github.com/google/go-github/v32/github"
"github.com/palantir/go-githubapp/githubapp"
"github.com/palantir/policy-bot/policy/common"
"github.com/palantir/policy-bot/pull"
"github.com/pkg/errors"
Expand All @@ -44,12 +45,16 @@ func (h *Details) ServeHTTP(w http.ResponseWriter, r *http.Request) error {

number, err := strconv.Atoi(pat.Param(r, "number"))
if err != nil {
http.Error(w, fmt.Sprintf("invalid pull request number: %v", err), http.StatusBadRequest)
http.Error(w, fmt.Sprintf("Invalid pull request number: %v", err), http.StatusBadRequest)
return nil
}

installation, err := h.Installations.GetByOwner(ctx, owner)
if err != nil {
if _, notFound := err.(githubapp.InstallationNotFound); notFound {
h.render404(w, owner, repo, number)
return nil
}
return err
}

Expand All @@ -72,22 +77,22 @@ func (h *Details) ServeHTTP(w http.ResponseWriter, r *http.Request) error {
level, _, err := client.Repositories.GetPermissionLevel(ctx, owner, repo, user)
if err != nil {
if isNotFound(err) {
http.Error(w, fmt.Sprintf("not found: %s/%s#%d", owner, repo, number), http.StatusNotFound)
h.render404(w, owner, repo, number)
return nil
}
return errors.Wrap(err, "failed to get user permission level")
}

// if the user does not have permission, pretend the repo/PR doesn't exist
if level.GetPermission() == "none" {
http.Error(w, fmt.Sprintf("not found: %s/%s#%d", owner, repo, number), http.StatusNotFound)
h.render404(w, owner, repo, number)
return nil
}

pr, _, err := client.PullRequests.Get(ctx, owner, repo, number)
if err != nil {
if isNotFound(err) {
http.Error(w, fmt.Sprintf("not found: %s/%s#%d", owner, repo, number), http.StatusNotFound)
h.render404(w, owner, repo, number)
return nil
}
return errors.Wrap(err, "failed to get pull request")
Expand Down Expand Up @@ -156,6 +161,14 @@ func (h *Details) render(w http.ResponseWriter, data interface{}) error {
return h.Templates.ExecuteTemplate(w, "details.html.tmpl", data)
}

func (h *Details) render404(w http.ResponseWriter, owner, repo string, number int) {
msg := fmt.Sprintf(
"Not Found: %s/%s#%d\n\nThe repository or pull request does not exist, you do not have permission, or policy-bot is not installed.",
owner, repo, number,
)
http.Error(w, msg, http.StatusNotFound)
}

func getPolicyURL(pr *github.PullRequest, config FetchedConfig) string {
base := pr.GetBase().GetRepo().GetHTMLURL()
if u, _ := url.Parse(base); u != nil {
Expand Down

0 comments on commit b80414e

Please sign in to comment.