Skip to content

Commit 9658610

Browse files
zeripathzeripath
authored
Return the full rejection message and errors in flash errors (#13221)
 Signed-off-by: Andrew Thornton <[email protected]>
1 parent 48a8009 commit 9658610

File tree

9 files changed

+128
-25
lines changed

9 files changed

+128
-25
lines changed

‎options/locale/locale_en-US.ini‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -870,9 +870,11 @@ editor.file_already_exists = A file named '%s' already exists in this repository
870870
editor.commit_empty_file_header = Commit an empty file
871871
editor.commit_empty_file_text = The file you're about to commit is empty. Proceed?
872872
editor.no_changes_to_show = There are no changes to show.
873-
editor.fail_to_update_file = Failed to update/create file '%s' with error: %v
873+
editor.fail_to_update_file = Failed to update/create file '%s'.
874+
editor.fail_to_update_file_summary = Error Message:
874875
editor.push_rejected_no_message = The change was rejected by the server without a message. Please check githooks.
875-
editor.push_rejected = The change was rejected by the server with the following message:<br>%s<br> Please check githooks.
876+
editor.push_rejected = The change was rejected by the server. Please check githooks.
877+
editor.push_rejected_summary = Full Rejection Message:
876878
editor.add_subdir = Add a directory…
877879
editor.unable_to_upload_files = Failed to upload files to '%s' with error: %v
878880
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)
12591261
pulls.squash_merge_pull_request = Squash and Merge
12601262
pulls.require_signed_wont_sign = The branch requires signed commits but this merge will not be signed
12611263
pulls.invalid_merge_option = You cannot use this merge option for this pull request.
1262-
pulls.merge_conflict = Merge Failed: There was a conflict whilst merging: %[1]s<br>%[2]s<br>Hint: Try a different strategy
1263-
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
1264+
pulls.merge_conflict = Merge Failed: There was a conflict whilst merging. Hint: Try a different strategy
1265+
pulls.merge_conflict_summary = Error Message
1266+
pulls.rebase_conflict = Merge Failed: There was a conflict whilst rebasing commit: %[1]s. Hint: Try a different strategy
1267+
pulls.rebase_conflict_summary = Error Message
1268+
; </summary><code>%[2]s<br>%[3]s</code></details>
12641269
pulls.unrelated_histories = Merge Failed: The merge head and base do not share a common history. Hint: Try a different strategy
12651270
pulls.merge_out_of_date = Merge Failed: Whilst generating the merge, the base was updated. Hint: Try again.
1266-
pulls.push_rejected = Merge Failed: The push was rejected with the following message:<br>%s<br>Review the githooks for this repository
1271+
pulls.push_rejected = Merge Failed: The push was rejected. Review the githooks for this repository.
1272+
pulls.push_rejected_summary = Full Rejection Message
12671273
pulls.push_rejected_no_message = Merge Failed: The push was rejected but there was no remote message.<br>Review the githooks for this repository
12681274
pulls.open_unmerged_pull_exists = `You cannot perform a reopen operation because there is a pending pull request (#%d) with identical properties.`
12691275
pulls.status_checking = Some checks are pending

‎routers/repo/branch.go‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,16 @@ func CreateBranch(ctx *context.Context, form auth.NewBranchForm) {
355355
if len(e.Message) == 0 {
356356
ctx.Flash.Error(ctx.Tr("repo.editor.push_rejected_no_message"))
357357
} else {
358-
ctx.Flash.Error(ctx.Tr("repo.editor.push_rejected", utils.SanitizeFlashErrorString(e.Message)))
358+
flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
359+
"Message": ctx.Tr("repo.editor.push_rejected"),
360+
"Summary": ctx.Tr("repo.editor.push_rejected_summary"),
361+
"Details": utils.SanitizeFlashErrorString(e.Message),
362+
})
363+
if err != nil {
364+
ctx.ServerError("UpdatePullRequest.HTMLString", err)
365+
return
366+
}
367+
ctx.Flash.Error(flashError)
359368
}
360369
ctx.Redirect(ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL())
361370
return

‎routers/repo/editor.go‎

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -293,10 +293,28 @@ func editFilePost(ctx *context.Context, form auth.EditRepoFileForm, isNewFile bo
293293
if len(errPushRej.Message) == 0 {
294294
ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected_no_message"), tplEditFile, &form)
295295
} else {
296-
ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected", utils.SanitizeFlashErrorString(errPushRej.Message)), tplEditFile, &form)
296+
flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
297+
"Message": ctx.Tr("repo.editor.push_rejected"),
298+
"Summary": ctx.Tr("repo.editor.push_rejected_summary"),
299+
"Details": utils.SanitizeFlashErrorString(errPushRej.Message),
300+
})
301+
if err != nil {
302+
ctx.ServerError("editFilePost.HTMLString", err)
303+
return
304+
}
305+
ctx.RenderWithErr(flashError, tplEditFile, &form)
297306
}
298307
} else {
299-
ctx.RenderWithErr(ctx.Tr("repo.editor.fail_to_update_file", form.TreePath, utils.SanitizeFlashErrorString(err.Error())), tplEditFile, &form)
308+
flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
309+
"Message": ctx.Tr("repo.editor.fail_to_update_file", form.TreePath),
310+
"Summary": ctx.Tr("repo.editor.fail_to_update_file_summary"),
311+
"Details": utils.SanitizeFlashErrorString(err.Error()),
312+
})
313+
if err != nil {
314+
ctx.ServerError("editFilePost.HTMLString", err)
315+
return
316+
}
317+
ctx.RenderWithErr(flashError, tplEditFile, &form)
300318
}
301319
}
302320

@@ -464,7 +482,16 @@ func DeleteFilePost(ctx *context.Context, form auth.DeleteRepoFileForm) {
464482
if len(errPushRej.Message) == 0 {
465483
ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected_no_message"), tplDeleteFile, &form)
466484
} else {
467-
ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected", utils.SanitizeFlashErrorString(errPushRej.Message)), tplDeleteFile, &form)
485+
flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
486+
"Message": ctx.Tr("repo.editor.push_rejected"),
487+
"Summary": ctx.Tr("repo.editor.push_rejected_summary"),
488+
"Details": utils.SanitizeFlashErrorString(errPushRej.Message),
489+
})
490+
if err != nil {
491+
ctx.ServerError("DeleteFilePost.HTMLString", err)
492+
return
493+
}
494+
ctx.RenderWithErr(flashError, tplDeleteFile, &form)
468495
}
469496
} else {
470497
ctx.ServerError("DeleteRepoFile", err)
@@ -656,7 +683,16 @@ func UploadFilePost(ctx *context.Context, form auth.UploadRepoFileForm) {
656683
if len(errPushRej.Message) == 0 {
657684
ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected_no_message"), tplUploadFile, &form)
658685
} else {
659-
ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected", utils.SanitizeFlashErrorString(errPushRej.Message)), tplUploadFile, &form)
686+
flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
687+
"Message": ctx.Tr("repo.editor.push_rejected"),
688+
"Summary": ctx.Tr("repo.editor.push_rejected_summary"),
689+
"Details": utils.SanitizeFlashErrorString(errPushRej.Message),
690+
})
691+
if err != nil {
692+
ctx.ServerError("UploadFilePost.HTMLString", err)
693+
return
694+
}
695+
ctx.RenderWithErr(flashError, tplUploadFile, &form)
660696
}
661697
} else {
662698
// os.ErrNotExist - upload file missing in the intervening time?!

‎routers/repo/pull.go‎

Lines changed: 50 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -723,7 +723,16 @@ func UpdatePullRequest(ctx *context.Context) {
723723
if err = pull_service.Update(issue.PullRequest, ctx.User, message); err != nil {
724724
if models.IsErrMergeConflicts(err) {
725725
conflictError := err.(models.ErrMergeConflicts)
726-
ctx.Flash.Error(ctx.Tr("repo.pulls.merge_conflict", utils.SanitizeFlashErrorString(conflictError.StdErr), utils.SanitizeFlashErrorString(conflictError.StdOut)))
726+
flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
727+
"Message": ctx.Tr("repo.pulls.merge_conflict"),
728+
"Summary": ctx.Tr("repo.pulls.merge_conflict_summary"),
729+
"Details": utils.SanitizeFlashErrorString(conflictError.StdErr) + "<br>" + utils.SanitizeFlashErrorString(conflictError.StdOut),
730+
})
731+
if err != nil {
732+
ctx.ServerError("UpdatePullRequest.HTMLString", err)
733+
return
734+
}
735+
ctx.Flash.Error(flashError)
727736
ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(issue.Index))
728737
return
729738
}
@@ -846,12 +855,30 @@ func MergePullRequest(ctx *context.Context, form auth.MergePullRequestForm) {
846855
return
847856
} else if models.IsErrMergeConflicts(err) {
848857
conflictError := err.(models.ErrMergeConflicts)
849-
ctx.Flash.Error(ctx.Tr("repo.pulls.merge_conflict", utils.SanitizeFlashErrorString(conflictError.StdErr), utils.SanitizeFlashErrorString(conflictError.StdOut)))
858+
flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
859+
"Message": ctx.Tr("repo.editor.merge_conflict"),
860+
"Summary": ctx.Tr("repo.editor.merge_conflict_summary"),
861+
"Details": utils.SanitizeFlashErrorString(conflictError.StdErr) + "<br>" + utils.SanitizeFlashErrorString(conflictError.StdOut),
862+
})
863+
if err != nil {
864+
ctx.ServerError("MergePullRequest.HTMLString", err)
865+
return
866+
}
867+
ctx.Flash.Error(flashError)
850868
ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index))
851869
return
852870
} else if models.IsErrRebaseConflicts(err) {
853871
conflictError := err.(models.ErrRebaseConflicts)
854-
ctx.Flash.Error(ctx.Tr("repo.pulls.rebase_conflict", utils.SanitizeFlashErrorString(conflictError.CommitSHA), utils.SanitizeFlashErrorString(conflictError.StdErr), utils.SanitizeFlashErrorString(conflictError.StdOut)))
872+
flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
873+
"Message": ctx.Tr("repo.editor.rebase_conflict", utils.SanitizeFlashErrorString(conflictError.CommitSHA)),
874+
"Summary": ctx.Tr("repo.editor.rebase_conflict_summary"),
875+
"Details": utils.SanitizeFlashErrorString(conflictError.StdErr) + "<br>" + utils.SanitizeFlashErrorString(conflictError.StdOut),
876+
})
877+
if err != nil {
878+
ctx.ServerError("MergePullRequest.HTMLString", err)
879+
return
880+
}
881+
ctx.Flash.Error(flashError)
855882
ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index))
856883
return
857884
} else if models.IsErrMergeUnrelatedHistories(err) {
@@ -871,7 +898,16 @@ func MergePullRequest(ctx *context.Context, form auth.MergePullRequestForm) {
871898
if len(message) == 0 {
872899
ctx.Flash.Error(ctx.Tr("repo.pulls.push_rejected_no_message"))
873900
} else {
874-
ctx.Flash.Error(ctx.Tr("repo.pulls.push_rejected", utils.SanitizeFlashErrorString(pushrejErr.Message)))
901+
flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
902+
"Message": ctx.Tr("repo.pulls.push_rejected"),
903+
"Summary": ctx.Tr("repo.pulls.push_rejected_summary"),
904+
"Details": utils.SanitizeFlashErrorString(pushrejErr.Message),
905+
})
906+
if err != nil {
907+
ctx.ServerError("MergePullRequest.HTMLString", err)
908+
return
909+
}
910+
ctx.Flash.Error(flashError)
875911
}
876912
ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index))
877913
return
@@ -986,7 +1022,16 @@ func CompareAndPullRequestPost(ctx *context.Context, form auth.CreateIssueForm)
9861022
if len(message) == 0 {
9871023
ctx.Flash.Error(ctx.Tr("repo.pulls.push_rejected_no_message"))
9881024
} else {
989-
ctx.Flash.Error(ctx.Tr("repo.pulls.push_rejected", utils.SanitizeFlashErrorString(pushrejErr.Message)))
1025+
flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
1026+
"Message": ctx.Tr("repo.pulls.push_rejected"),
1027+
"Summary": ctx.Tr("repo.pulls.push_rejected_summary"),
1028+
"Details": utils.SanitizeFlashErrorString(pushrejErr.Message),
1029+
})
1030+
if err != nil {
1031+
ctx.ServerError("CompareAndPullRequest.HTMLString", err)
1032+
return
1033+
}
1034+
ctx.Flash.Error(flashError)
9901035
}
9911036
ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pullIssue.Index))
9921037
return

‎routers/repo/repo.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ import (
2424
)
2525

2626
const (
27-
tplCreate base.TplName = "repo/create"
27+
tplCreate base.TplName = "repo/create"
28+
tplAlertDetails base.TplName = "base/alert_details"
2829
)
2930

3031
// MustBeNotEmpty render when a repo is a empty git dir

‎routers/utils/utils.go‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,6 @@ func IsValidSlackChannel(channelName string) bool {
4141

4242
// SanitizeFlashErrorString will sanitize a flash error string
4343
func SanitizeFlashErrorString(x string) string {
44-
runes := []rune(x)
45-
46-
if len(runes) > 512 {
47-
x = "..." + string(runes[len(runes)-512:])
48-
}
49-
5044
return strings.ReplaceAll(html.EscapeString(x), "\n", "<br>")
5145
}
5246

‎templates/base/alert.tmpl‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
{{if .Flash.ErrorMsg}}
2-
<div class="ui negative message">
2+
<div class="ui negative message flash-error">
33
<p>{{.Flash.ErrorMsg | Str2html}}</p>
44
</div>
55
{{end}}
66
{{if .Flash.SuccessMsg}}
7-
<div class="ui positive message">
7+
<div class="ui positive message flash-success">
88
<p>{{.Flash.SuccessMsg | Str2html}}</p>
99
</div>
1010
{{end}}
1111
{{if .Flash.InfoMsg}}
12-
<div class="ui info message">
12+
<div class="ui info message flash-info">
1313
<p>{{.Flash.InfoMsg | Str2html}}</p>
1414
</div>
1515
{{end}}

‎templates/base/alert_details.tmpl‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{{.Message}}
2+
<details>
3+
<summary>{{.Summary}}</summary>
4+
<code>
5+
{{.Details | Str2html}}
6+
</code>
7+
</details>

‎web_src/less/_base.less‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1268,3 +1268,8 @@ table th[data-sortt-desc] {
12681268
.ui.header > .ui.label.compact {
12691269
margin-top: inherit;
12701270
}
1271+
1272+
.flash-error details code {
1273+
display: block;
1274+
text-align: left;
1275+
}

0 commit comments

Comments
 (0)