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

Signed-off-by: Andrew Thornton <art27@cantab.net>
This commit is contained in:
zeripath 2020-10-21 00:50:10 +01:00 committed by GitHub
parent 48a80096bb
commit 965861043a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 128 additions and 25 deletions

View File

@ -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_header = Commit an empty file
editor.commit_empty_file_text = The file you're about to commit is empty. Proceed? 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.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_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.add_subdir = Add a directory…
editor.unable_to_upload_files = Failed to upload files to '%s' with error: %v 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. editor.upload_file_is_locked = File '%s' is locked by %s.
@ -1259,11 +1261,15 @@ pulls.rebase_merge_commit_pull_request = Rebase and Merge (--no-ff)
pulls.squash_merge_pull_request = Squash and Merge 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.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.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.merge_conflict = Merge Failed: There was a conflict whilst merging. 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_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.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.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.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.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 pulls.status_checking = Some checks are pending

View File

@ -355,7 +355,16 @@ func CreateBranch(ctx *context.Context, form auth.NewBranchForm) {
if len(e.Message) == 0 { if len(e.Message) == 0 {
ctx.Flash.Error(ctx.Tr("repo.editor.push_rejected_no_message")) ctx.Flash.Error(ctx.Tr("repo.editor.push_rejected_no_message"))
} else { } 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()) ctx.Redirect(ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL())
return return

View File

@ -293,10 +293,28 @@ func editFilePost(ctx *context.Context, form auth.EditRepoFileForm, isNewFile bo
if len(errPushRej.Message) == 0 { if len(errPushRej.Message) == 0 {
ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected_no_message"), tplEditFile, &form) ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected_no_message"), tplEditFile, &form)
} else { } 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 { } 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)
} }
} }
@ -464,7 +482,16 @@ func DeleteFilePost(ctx *context.Context, form auth.DeleteRepoFileForm) {
if len(errPushRej.Message) == 0 { if len(errPushRej.Message) == 0 {
ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected_no_message"), tplDeleteFile, &form) ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected_no_message"), tplDeleteFile, &form)
} else { } 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 { } else {
ctx.ServerError("DeleteRepoFile", err) ctx.ServerError("DeleteRepoFile", err)
@ -656,7 +683,16 @@ func UploadFilePost(ctx *context.Context, form auth.UploadRepoFileForm) {
if len(errPushRej.Message) == 0 { if len(errPushRej.Message) == 0 {
ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected_no_message"), tplUploadFile, &form) ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected_no_message"), tplUploadFile, &form)
} else { } 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 { } else {
// os.ErrNotExist - upload file missing in the intervening time?! // os.ErrNotExist - upload file missing in the intervening time?!

View File

@ -723,7 +723,16 @@ func UpdatePullRequest(ctx *context.Context) {
if err = pull_service.Update(issue.PullRequest, ctx.User, message); err != nil { if err = pull_service.Update(issue.PullRequest, ctx.User, message); err != nil {
if models.IsErrMergeConflicts(err) { if models.IsErrMergeConflicts(err) {
conflictError := err.(models.ErrMergeConflicts) 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)) ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(issue.Index))
return return
} }
@ -846,12 +855,30 @@ func MergePullRequest(ctx *context.Context, form auth.MergePullRequestForm) {
return return
} else if models.IsErrMergeConflicts(err) { } else if models.IsErrMergeConflicts(err) {
conflictError := err.(models.ErrMergeConflicts) 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)) ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index))
return return
} else if models.IsErrRebaseConflicts(err) { } else if models.IsErrRebaseConflicts(err) {
conflictError := err.(models.ErrRebaseConflicts) 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.editor.rebase_conflict", utils.SanitizeFlashErrorString(conflictError.CommitSHA)),
"Summary": ctx.Tr("repo.editor.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)) ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index))
return return
} else if models.IsErrMergeUnrelatedHistories(err) { } else if models.IsErrMergeUnrelatedHistories(err) {
@ -871,7 +898,16 @@ func MergePullRequest(ctx *context.Context, form auth.MergePullRequestForm) {
if len(message) == 0 { if len(message) == 0 {
ctx.Flash.Error(ctx.Tr("repo.pulls.push_rejected_no_message")) ctx.Flash.Error(ctx.Tr("repo.pulls.push_rejected_no_message"))
} else { } 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)) ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index))
return return
@ -986,7 +1022,16 @@ func CompareAndPullRequestPost(ctx *context.Context, form auth.CreateIssueForm)
if len(message) == 0 { if len(message) == 0 {
ctx.Flash.Error(ctx.Tr("repo.pulls.push_rejected_no_message")) ctx.Flash.Error(ctx.Tr("repo.pulls.push_rejected_no_message"))
} else { } 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)) ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pullIssue.Index))
return return

View File

@ -24,7 +24,8 @@ import (
) )
const ( 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 // MustBeNotEmpty render when a repo is a empty git dir

View File

@ -41,12 +41,6 @@ func IsValidSlackChannel(channelName string) bool {
// SanitizeFlashErrorString will sanitize a flash error string // SanitizeFlashErrorString will sanitize a flash error string
func SanitizeFlashErrorString(x string) 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>") return strings.ReplaceAll(html.EscapeString(x), "\n", "<br>")
} }

View File

@ -1,15 +1,15 @@
{{if .Flash.ErrorMsg}} {{if .Flash.ErrorMsg}}
<div class="ui negative message"> <div class="ui negative message flash-error">
<p>{{.Flash.ErrorMsg | Str2html}}</p> <p>{{.Flash.ErrorMsg | Str2html}}</p>
</div> </div>
{{end}} {{end}}
{{if .Flash.SuccessMsg}} {{if .Flash.SuccessMsg}}
<div class="ui positive message"> <div class="ui positive message flash-success">
<p>{{.Flash.SuccessMsg | Str2html}}</p> <p>{{.Flash.SuccessMsg | Str2html}}</p>
</div> </div>
{{end}} {{end}}
{{if .Flash.InfoMsg}} {{if .Flash.InfoMsg}}
<div class="ui info message"> <div class="ui info message flash-info">
<p>{{.Flash.InfoMsg | Str2html}}</p> <p>{{.Flash.InfoMsg | Str2html}}</p>
</div> </div>
{{end}} {{end}}

View File

@ -0,0 +1,7 @@
{{.Message}}
<details>
<summary>{{.Summary}}</summary>
<code>
{{.Details | Str2html}}
</code>
</details>

View File

@ -1268,3 +1268,8 @@ table th[data-sortt-desc] {
.ui.header > .ui.label.compact { .ui.header > .ui.label.compact {
margin-top: inherit; margin-top: inherit;
} }
.flash-error details code {
display: block;
text-align: left;
}