From 965861043ae4daf7ae50460edf5533e52980bdb1 Mon Sep 17 00:00:00 2001 From: zeripath Date: Wed, 21 Oct 2020 00:50:10 +0100 Subject: [PATCH] Return the full rejection message and errors in flash errors (#13221) Signed-off-by: Andrew Thornton --- options/locale/locale_en-US.ini | 16 ++++++--- routers/repo/branch.go | 11 ++++++- routers/repo/editor.go | 44 ++++++++++++++++++++++--- routers/repo/pull.go | 55 ++++++++++++++++++++++++++++--- routers/repo/repo.go | 3 +- routers/utils/utils.go | 6 ---- templates/base/alert.tmpl | 6 ++-- templates/base/alert_details.tmpl | 7 ++++ web_src/less/_base.less | 5 +++ 9 files changed, 128 insertions(+), 25 deletions(-) create mode 100644 templates/base/alert_details.tmpl diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 16d3874e1..18c621db6 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -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:
%s
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. @@ -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
%[2]s
Hint: Try a different strategy -pulls.rebase_conflict = Merge Failed: There was a conflict whilst rebasing commit: %[1]s
%[2]s
%[3]s
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 +; %[2]s
%[3]s 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:
%s
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.
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 diff --git a/routers/repo/branch.go b/routers/repo/branch.go index 0ca77cbf6..cd18f6677 100644 --- a/routers/repo/branch.go +++ b/routers/repo/branch.go @@ -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 diff --git a/routers/repo/editor.go b/routers/repo/editor.go index a08c7193f..1ee557a4f 100644 --- a/routers/repo/editor.go +++ b/routers/repo/editor.go @@ -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) } } @@ -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) @@ -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?! diff --git a/routers/repo/pull.go b/routers/repo/pull.go index 0ae126215..441bf9dbf 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -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) + "
" + 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 } @@ -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) + "
" + 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.editor.rebase_conflict", utils.SanitizeFlashErrorString(conflictError.CommitSHA)), + "Summary": ctx.Tr("repo.editor.rebase_conflict_summary"), + "Details": utils.SanitizeFlashErrorString(conflictError.StdErr) + "
" + 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) { @@ -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 @@ -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 diff --git a/routers/repo/repo.go b/routers/repo/repo.go index 883d30a5d..742c952f6 100644 --- a/routers/repo/repo.go +++ b/routers/repo/repo.go @@ -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 diff --git a/routers/utils/utils.go b/routers/utils/utils.go index d8060ff07..f15bc1e62 100644 --- a/routers/utils/utils.go +++ b/routers/utils/utils.go @@ -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", "
") } diff --git a/templates/base/alert.tmpl b/templates/base/alert.tmpl index 61b99486e..cf886f529 100644 --- a/templates/base/alert.tmpl +++ b/templates/base/alert.tmpl @@ -1,15 +1,15 @@ {{if .Flash.ErrorMsg}} -
+

{{.Flash.ErrorMsg | Str2html}}

{{end}} {{if .Flash.SuccessMsg}} -
+

{{.Flash.SuccessMsg | Str2html}}

{{end}} {{if .Flash.InfoMsg}} -
+

{{.Flash.InfoMsg | Str2html}}

{{end}} diff --git a/templates/base/alert_details.tmpl b/templates/base/alert_details.tmpl new file mode 100644 index 000000000..38a2721bf --- /dev/null +++ b/templates/base/alert_details.tmpl @@ -0,0 +1,7 @@ +{{.Message}} +
+ {{.Summary}} + + {{.Details | Str2html}} + +
diff --git a/web_src/less/_base.less b/web_src/less/_base.less index c25af2126..81111d82f 100644 --- a/web_src/less/_base.less +++ b/web_src/less/_base.less @@ -1268,3 +1268,8 @@ table th[data-sortt-desc] { .ui.header > .ui.label.compact { margin-top: inherit; } + +.flash-error details code { + display: block; + text-align: left; +}