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

Return the full rejection message and errors in flash errors (#13221) #13237

Merged
merged 2 commits into from
Oct 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -870,9 +870,11 @@ editor.file_already_exists = A file named '%s' already exists in this repository
editor.commit_empty_file_header = Commit an empty file
editor.commit_empty_file_text = The file you're about to commit is empty. Proceed?
editor.no_changes_to_show = There are no changes to show.
editor.fail_to_update_file = Failed to update/create file '%s' with error: %v
editor.fail_to_update_file = Failed to update/create file '%s'.
editor.fail_to_update_file_summary = Error Message:
editor.push_rejected_no_message = The change was rejected by the server without a message. Please check githooks.
editor.push_rejected = The change was rejected by the server with the following message:<br>%s<br> Please check githooks.
editor.push_rejected = The change was rejected by the server. Please check githooks.
editor.push_rejected_summary = Full Rejection Message:
editor.add_subdir = Add a directory…
editor.unable_to_upload_files = Failed to upload files to '%s' with error: %v
editor.upload_file_is_locked = File '%s' is locked by %s.
Expand Down Expand Up @@ -1259,11 +1261,15 @@ pulls.rebase_merge_commit_pull_request = Rebase and Merge (--no-ff)
pulls.squash_merge_pull_request = Squash and Merge
pulls.require_signed_wont_sign = The branch requires signed commits but this merge will not be signed
pulls.invalid_merge_option = You cannot use this merge option for this pull request.
pulls.merge_conflict = Merge Failed: There was a conflict whilst merging: %[1]s<br>%[2]s<br>Hint: Try a different strategy
pulls.rebase_conflict = Merge Failed: There was a conflict whilst rebasing commit: %[1]s<br>%[2]s<br>%[3]s<br>Hint:Try a different strategy
pulls.merge_conflict = Merge Failed: There was a conflict whilst merging. Hint: Try a different strategy
pulls.merge_conflict_summary = Error Message
pulls.rebase_conflict = Merge Failed: There was a conflict whilst rebasing commit: %[1]s. Hint: Try a different strategy
pulls.rebase_conflict_summary = Error Message
; </summary><code>%[2]s<br>%[3]s</code></details>
pulls.unrelated_histories = Merge Failed: The merge head and base do not share a common history. Hint: Try a different strategy
pulls.merge_out_of_date = Merge Failed: Whilst generating the merge, the base was updated. Hint: Try again.
pulls.push_rejected = Merge Failed: The push was rejected with the following message:<br>%s<br>Review the githooks for this repository
pulls.push_rejected = Merge Failed: The push was rejected. Review the githooks for this repository.
pulls.push_rejected_summary = Full Rejection Message
pulls.push_rejected_no_message = Merge Failed: The push was rejected but there was no remote message.<br>Review the githooks for this repository
pulls.open_unmerged_pull_exists = `You cannot perform a reopen operation because there is a pending pull request (#%d) with identical properties.`
pulls.status_checking = Some checks are pending
Expand Down
11 changes: 10 additions & 1 deletion routers/repo/branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,16 @@ func CreateBranch(ctx *context.Context, form auth.NewBranchForm) {
if len(e.Message) == 0 {
ctx.Flash.Error(ctx.Tr("repo.editor.push_rejected_no_message"))
} else {
ctx.Flash.Error(ctx.Tr("repo.editor.push_rejected", utils.SanitizeFlashErrorString(e.Message)))
flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
"Message": ctx.Tr("repo.editor.push_rejected"),
"Summary": ctx.Tr("repo.editor.push_rejected_summary"),
"Details": utils.SanitizeFlashErrorString(e.Message),
})
if err != nil {
ctx.ServerError("UpdatePullRequest.HTMLString", err)
return
}
ctx.Flash.Error(flashError)
}
ctx.Redirect(ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL())
return
Expand Down
44 changes: 40 additions & 4 deletions routers/repo/editor.go
Original file line number Diff line number Diff line change
Expand Up @@ -293,10 +293,28 @@ func editFilePost(ctx *context.Context, form auth.EditRepoFileForm, isNewFile bo
if len(errPushRej.Message) == 0 {
ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected_no_message"), tplEditFile, &form)
} else {
ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected", utils.SanitizeFlashErrorString(errPushRej.Message)), tplEditFile, &form)
flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
"Message": ctx.Tr("repo.editor.push_rejected"),
"Summary": ctx.Tr("repo.editor.push_rejected_summary"),
"Details": utils.SanitizeFlashErrorString(errPushRej.Message),
})
if err != nil {
ctx.ServerError("editFilePost.HTMLString", err)
return
}
ctx.RenderWithErr(flashError, tplEditFile, &form)
}
} else {
ctx.RenderWithErr(ctx.Tr("repo.editor.fail_to_update_file", form.TreePath, utils.SanitizeFlashErrorString(err.Error())), tplEditFile, &form)
flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
"Message": ctx.Tr("repo.editor.fail_to_update_file", form.TreePath),
"Summary": ctx.Tr("repo.editor.fail_to_update_file_summary"),
"Details": utils.SanitizeFlashErrorString(err.Error()),
})
if err != nil {
ctx.ServerError("editFilePost.HTMLString", err)
return
}
ctx.RenderWithErr(flashError, tplEditFile, &form)
}
}

Expand Down Expand Up @@ -464,7 +482,16 @@ func DeleteFilePost(ctx *context.Context, form auth.DeleteRepoFileForm) {
if len(errPushRej.Message) == 0 {
ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected_no_message"), tplDeleteFile, &form)
} else {
ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected", utils.SanitizeFlashErrorString(errPushRej.Message)), tplDeleteFile, &form)
flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
"Message": ctx.Tr("repo.editor.push_rejected"),
"Summary": ctx.Tr("repo.editor.push_rejected_summary"),
"Details": utils.SanitizeFlashErrorString(errPushRej.Message),
})
if err != nil {
ctx.ServerError("DeleteFilePost.HTMLString", err)
return
}
ctx.RenderWithErr(flashError, tplDeleteFile, &form)
}
} else {
ctx.ServerError("DeleteRepoFile", err)
Expand Down Expand Up @@ -656,7 +683,16 @@ func UploadFilePost(ctx *context.Context, form auth.UploadRepoFileForm) {
if len(errPushRej.Message) == 0 {
ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected_no_message"), tplUploadFile, &form)
} else {
ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected", utils.SanitizeFlashErrorString(errPushRej.Message)), tplUploadFile, &form)
flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
"Message": ctx.Tr("repo.editor.push_rejected"),
"Summary": ctx.Tr("repo.editor.push_rejected_summary"),
"Details": utils.SanitizeFlashErrorString(errPushRej.Message),
})
if err != nil {
ctx.ServerError("UploadFilePost.HTMLString", err)
return
}
ctx.RenderWithErr(flashError, tplUploadFile, &form)
}
} else {
// os.ErrNotExist - upload file missing in the intervening time?!
Expand Down
55 changes: 50 additions & 5 deletions routers/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -723,7 +723,16 @@ func UpdatePullRequest(ctx *context.Context) {
if err = pull_service.Update(issue.PullRequest, ctx.User, message); err != nil {
if models.IsErrMergeConflicts(err) {
conflictError := err.(models.ErrMergeConflicts)
ctx.Flash.Error(ctx.Tr("repo.pulls.merge_conflict", utils.SanitizeFlashErrorString(conflictError.StdErr), utils.SanitizeFlashErrorString(conflictError.StdOut)))
flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
"Message": ctx.Tr("repo.pulls.merge_conflict"),
"Summary": ctx.Tr("repo.pulls.merge_conflict_summary"),
"Details": utils.SanitizeFlashErrorString(conflictError.StdErr) + "<br>" + utils.SanitizeFlashErrorString(conflictError.StdOut),
})
if err != nil {
ctx.ServerError("UpdatePullRequest.HTMLString", err)
return
}
ctx.Flash.Error(flashError)
ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(issue.Index))
return
}
Expand Down Expand Up @@ -846,12 +855,30 @@ func MergePullRequest(ctx *context.Context, form auth.MergePullRequestForm) {
return
} else if models.IsErrMergeConflicts(err) {
conflictError := err.(models.ErrMergeConflicts)
ctx.Flash.Error(ctx.Tr("repo.pulls.merge_conflict", utils.SanitizeFlashErrorString(conflictError.StdErr), utils.SanitizeFlashErrorString(conflictError.StdOut)))
flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
"Message": ctx.Tr("repo.editor.merge_conflict"),
"Summary": ctx.Tr("repo.editor.merge_conflict_summary"),
"Details": utils.SanitizeFlashErrorString(conflictError.StdErr) + "<br>" + utils.SanitizeFlashErrorString(conflictError.StdOut),
})
if err != nil {
ctx.ServerError("MergePullRequest.HTMLString", err)
return
}
ctx.Flash.Error(flashError)
ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index))
return
} else if models.IsErrRebaseConflicts(err) {
conflictError := err.(models.ErrRebaseConflicts)
ctx.Flash.Error(ctx.Tr("repo.pulls.rebase_conflict", utils.SanitizeFlashErrorString(conflictError.CommitSHA), utils.SanitizeFlashErrorString(conflictError.StdErr), utils.SanitizeFlashErrorString(conflictError.StdOut)))
flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
"Message": ctx.Tr("repo.pulls.rebase_conflict", utils.SanitizeFlashErrorString(conflictError.CommitSHA)),
"Summary": ctx.Tr("repo.pulls.rebase_conflict_summary"),
"Details": utils.SanitizeFlashErrorString(conflictError.StdErr) + "<br>" + utils.SanitizeFlashErrorString(conflictError.StdOut),
})
if err != nil {
ctx.ServerError("MergePullRequest.HTMLString", err)
return
}
ctx.Flash.Error(flashError)
ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index))
return
} else if models.IsErrMergeUnrelatedHistories(err) {
Expand All @@ -871,7 +898,16 @@ func MergePullRequest(ctx *context.Context, form auth.MergePullRequestForm) {
if len(message) == 0 {
ctx.Flash.Error(ctx.Tr("repo.pulls.push_rejected_no_message"))
} else {
ctx.Flash.Error(ctx.Tr("repo.pulls.push_rejected", utils.SanitizeFlashErrorString(pushrejErr.Message)))
flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
"Message": ctx.Tr("repo.pulls.push_rejected"),
"Summary": ctx.Tr("repo.pulls.push_rejected_summary"),
"Details": utils.SanitizeFlashErrorString(pushrejErr.Message),
})
if err != nil {
ctx.ServerError("MergePullRequest.HTMLString", err)
return
}
ctx.Flash.Error(flashError)
}
ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index))
return
Expand Down Expand Up @@ -986,7 +1022,16 @@ func CompareAndPullRequestPost(ctx *context.Context, form auth.CreateIssueForm)
if len(message) == 0 {
ctx.Flash.Error(ctx.Tr("repo.pulls.push_rejected_no_message"))
} else {
ctx.Flash.Error(ctx.Tr("repo.pulls.push_rejected", utils.SanitizeFlashErrorString(pushrejErr.Message)))
flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
"Message": ctx.Tr("repo.pulls.push_rejected"),
"Summary": ctx.Tr("repo.pulls.push_rejected_summary"),
"Details": utils.SanitizeFlashErrorString(pushrejErr.Message),
})
if err != nil {
ctx.ServerError("CompareAndPullRequest.HTMLString", err)
return
}
ctx.Flash.Error(flashError)
}
ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pullIssue.Index))
return
Expand Down
3 changes: 2 additions & 1 deletion routers/repo/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ import (
)

const (
tplCreate base.TplName = "repo/create"
tplCreate base.TplName = "repo/create"
tplAlertDetails base.TplName = "base/alert_details"
)

// MustBeNotEmpty render when a repo is a empty git dir
Expand Down
6 changes: 0 additions & 6 deletions routers/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,6 @@ func IsValidSlackChannel(channelName string) bool {

// SanitizeFlashErrorString will sanitize a flash error string
func SanitizeFlashErrorString(x string) string {
runes := []rune(x)

if len(runes) > 512 {
x = "..." + string(runes[len(runes)-512:])
}

return strings.ReplaceAll(html.EscapeString(x), "\n", "<br>")
}

Expand Down
6 changes: 3 additions & 3 deletions templates/base/alert.tmpl
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
{{if .Flash.ErrorMsg}}
<div class="ui negative message">
<div class="ui negative message flash-error">
<p>{{.Flash.ErrorMsg | Str2html}}</p>
</div>
{{end}}
{{if .Flash.SuccessMsg}}
<div class="ui positive message">
<div class="ui positive message flash-success">
<p>{{.Flash.SuccessMsg | Str2html}}</p>
</div>
{{end}}
{{if .Flash.InfoMsg}}
<div class="ui info message">
<div class="ui info message flash-info">
<p>{{.Flash.InfoMsg | Str2html}}</p>
</div>
{{end}}
7 changes: 7 additions & 0 deletions templates/base/alert_details.tmpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{{.Message}}
<details>
<summary>{{.Summary}}</summary>
<code>
{{.Details | Str2html}}
</code>
</details>
5 changes: 5 additions & 0 deletions web_src/less/_base.less
Original file line number Diff line number Diff line change
Expand Up @@ -1280,3 +1280,8 @@ table th[data-sortt-desc] {
.ui.header > .ui.label.compact {
margin-top: inherit;
}

.flash-error details code {
display: block;
text-align: left;
}