mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2025-12-07 14:09:47 +00:00
i18n: translate Actions PreExecutionError for viewer (#10267)
Identified in code review https://codeberg.org/forgejo/forgejo/pulls/10244#issuecomment-8576643, the `PreExecutionError` field in `ActionRun` isn't well implemented as it translates the error at action runtime rather than later when the action is viewed in the UI. This PR adds an error code and error details column that can be more correctly translated. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10267 Reviewed-by: 0ko <0ko@noreply.codeberg.org> Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net> Co-committed-by: Mathieu Fenniak <mathieu@fenniak.net>
This commit is contained in:
parent
6edfeb60f9
commit
993da59ad4
10 changed files with 165 additions and 28 deletions
41
models/actions/pre_execution_errors.go
Normal file
41
models/actions/pre_execution_errors.go
Normal file
|
|
@ -0,0 +1,41 @@
|
|||
// Copyright 2025 The Forgejo Authors. All rights reserved.
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
|
||||
package actions
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
|
||||
"forgejo.org/modules/translation"
|
||||
)
|
||||
|
||||
type PreExecutionError int64
|
||||
|
||||
// PreExecutionError values are stored in the database in ActionRun.PreExecutionError and therefore values can't be
|
||||
// reordered or changed without a database migration. Translation arguments are stored in the database in
|
||||
// PreExecutionErrorDetails, and so they can't be changed or reordered without creating a migration or a new error code
|
||||
// to represent the new argument details.
|
||||
const (
|
||||
ErrorCodeEventDetectionError PreExecutionError = iota + 1
|
||||
ErrorCodeJobParsingError
|
||||
ErrorCodePersistentIncompleteMatrix
|
||||
)
|
||||
|
||||
func TranslatePreExecutionError(lang translation.Locale, run *ActionRun) string {
|
||||
if run.PreExecutionError != "" {
|
||||
// legacy: Forgejo v13 stored value pre-translated, preventing correct translation to active user
|
||||
return run.PreExecutionError
|
||||
}
|
||||
|
||||
switch run.PreExecutionErrorCode {
|
||||
case 0:
|
||||
return ""
|
||||
case ErrorCodeEventDetectionError:
|
||||
return lang.TrString("actions.workflow.event_detection_error", run.PreExecutionErrorDetails...)
|
||||
case ErrorCodeJobParsingError:
|
||||
return lang.TrString("actions.workflow.job_parsing_error", run.PreExecutionErrorDetails...)
|
||||
case ErrorCodePersistentIncompleteMatrix:
|
||||
return lang.TrString("actions.workflow.persistent_incomplete_matrix", run.PreExecutionErrorDetails...)
|
||||
}
|
||||
return fmt.Sprintf("<unsupported error: code=%v details=%#v", run.PreExecutionErrorCode, run.PreExecutionErrorDetails)
|
||||
}
|
||||
64
models/actions/pre_execution_errors_test.go
Normal file
64
models/actions/pre_execution_errors_test.go
Normal file
|
|
@ -0,0 +1,64 @@
|
|||
// Copyright 2025 The Forgejo Authors. All rights reserved.
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
|
||||
package actions
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"forgejo.org/modules/translation"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
func TestTranslatePreExecutionError(t *testing.T) {
|
||||
translation.InitLocales(t.Context())
|
||||
lang := translation.NewLocale("en-US")
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
run *ActionRun
|
||||
expected string
|
||||
}{
|
||||
{
|
||||
name: "legacy",
|
||||
run: &ActionRun{PreExecutionError: "legacy message"},
|
||||
expected: "legacy message",
|
||||
},
|
||||
{
|
||||
name: "no error",
|
||||
run: &ActionRun{},
|
||||
expected: "",
|
||||
},
|
||||
{
|
||||
name: "ErrorCodeEventDetectionError",
|
||||
run: &ActionRun{
|
||||
PreExecutionErrorCode: ErrorCodeEventDetectionError,
|
||||
PreExecutionErrorDetails: []any{"inner error message"},
|
||||
},
|
||||
expected: "Unable to parse supported events in workflow: inner error message",
|
||||
},
|
||||
{
|
||||
name: "ErrorCodeJobParsingError",
|
||||
run: &ActionRun{
|
||||
PreExecutionErrorCode: ErrorCodeJobParsingError,
|
||||
PreExecutionErrorDetails: []any{"inner error message"},
|
||||
},
|
||||
expected: "Unable to parse jobs in workflow: inner error message",
|
||||
},
|
||||
{
|
||||
name: "ErrorCodePersistentIncompleteMatrix",
|
||||
run: &ActionRun{
|
||||
PreExecutionErrorCode: ErrorCodePersistentIncompleteMatrix,
|
||||
PreExecutionErrorDetails: []any{"blocked_job", "needs-1, needs-2"},
|
||||
},
|
||||
expected: "Unable to evaluate `strategy.matrix` of job blocked_job due to a `needs` expression that was invalid. It may reference a job that is not in it's 'needs' list (needs-1, needs-2), or an output that doesn't exist on one of those jobs.",
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
err := TranslatePreExecutionError(lang, tt.run)
|
||||
assert.Equal(t, tt.expected, err)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
@ -78,7 +78,10 @@ type ActionRun struct {
|
|||
ConcurrencyGroup string `xorm:"'concurrency_group' index(concurrency)"`
|
||||
ConcurrencyType ConcurrencyMode
|
||||
|
||||
PreExecutionError string `xorm:"LONGTEXT"` // used to report errors that blocked execution of a workflow
|
||||
// used to report errors that blocked execution of a workflow
|
||||
PreExecutionError string `xorm:"LONGTEXT"` // deprecated: replaced with PreExecutionErrorCode and PreExecutionErrorDetails for better i18n
|
||||
PreExecutionErrorCode PreExecutionError
|
||||
PreExecutionErrorDetails []any `xorm:"JSON LONGTEXT"`
|
||||
}
|
||||
|
||||
func init() {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,24 @@
|
|||
// Copyright 2025 The Forgejo Authors. All rights reserved.
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
|
||||
package forgejo_migrations
|
||||
|
||||
import (
|
||||
"xorm.io/xorm"
|
||||
)
|
||||
|
||||
func init() {
|
||||
registerMigration(&Migration{
|
||||
Description: "add PreExecutionErrorCode & PreExecutionErrorDetails to action_run",
|
||||
Upgrade: addActionRunPreExecutionErrorCode,
|
||||
})
|
||||
}
|
||||
|
||||
func addActionRunPreExecutionErrorCode(x *xorm.Engine) error {
|
||||
type ActionRun struct {
|
||||
PreExecutionErrorCode int64
|
||||
PreExecutionErrorDetails []any `xorm:"JSON LONGTEXT"`
|
||||
}
|
||||
_, err := x.SyncWithOptions(xorm.SyncOptions{IgnoreDropIndices: true}, new(ActionRun))
|
||||
return err
|
||||
}
|
||||
|
|
@ -198,6 +198,7 @@
|
|||
"actions.runs.view_most_recent_run": "View most recent run",
|
||||
"actions.workflow.job_parsing_error": "Unable to parse jobs in workflow: %v",
|
||||
"actions.workflow.event_detection_error": "Unable to parse supported events in workflow: %v",
|
||||
"actions.workflow.persistent_incomplete_matrix": "Unable to evaluate `strategy.matrix` of job %[1]s due to a `needs` expression that was invalid. It may reference a job that is not in it's 'needs' list (%[2]s), or an output that doesn't exist on one of those jobs.",
|
||||
"actions.workflow.pre_execution_error": "Workflow was not executed due to an error that blocked the execution attempt.",
|
||||
"pulse.n_active_issues": {
|
||||
"one": "%s active issue",
|
||||
|
|
|
|||
|
|
@ -286,7 +286,7 @@ func getViewResponse(ctx *context_module.Context, req *ViewRequest, runIndex, jo
|
|||
resp.State.Run.CanDeleteArtifact = run.Status.IsDone() && ctx.Repo.CanWrite(unit.TypeActions)
|
||||
resp.State.Run.Jobs = make([]*ViewJob, 0, len(jobs)) // marshal to '[]' instead of 'null' in json
|
||||
resp.State.Run.Status = run.Status.String()
|
||||
resp.State.Run.PreExecutionError = run.PreExecutionError
|
||||
resp.State.Run.PreExecutionError = actions_model.TranslatePreExecutionError(ctx.Locale, run)
|
||||
|
||||
// It's possible for the run to be marked with a finalized status (eg. failure) because of a single job within the
|
||||
// run; eg. one job fails, the run fails. But other jobs can still be running. The frontend RepoActionView uses the
|
||||
|
|
|
|||
|
|
@ -239,18 +239,15 @@ func tryHandleIncompleteMatrix(ctx context.Context, blockedJob *actions_model.Ac
|
|||
// This is an error that needs to be reported back to the user for them to correct their workflow, so we
|
||||
// slip this notification into PreExecutionError.
|
||||
|
||||
// This string isn't translated because PreExecutionError needs a design update -- it only stores a single
|
||||
// string that is displayed to all users irrespective of their language. We could guess at the the language
|
||||
// based upon the triggering user of the workflow, but it would just be a rough guess, and it would expose a
|
||||
// user's personal configuration (their language).
|
||||
run := blockedJob.Run
|
||||
run.PreExecutionError = fmt.Sprintf(
|
||||
"Unable to evaluate `strategy.matrix` of job %[1]s due to a `needs` expression that was invalid. It may reference a job that is not in it's 'needs' list (%[2]s), or an output that doesn't exist on one of those jobs.",
|
||||
run.PreExecutionErrorCode = actions_model.ErrorCodePersistentIncompleteMatrix
|
||||
run.PreExecutionErrorDetails = []any{
|
||||
blockedJob.JobID,
|
||||
strings.Join(blockedJob.Needs, ", "),
|
||||
)
|
||||
}
|
||||
run.Status = actions_model.StatusFailure
|
||||
err = actions_model.UpdateRunWithoutNotification(ctx, run, "pre_execution_error", "status")
|
||||
err = actions_model.UpdateRunWithoutNotification(ctx, run,
|
||||
"pre_execution_error_code", "pre_execution_error_details", "status")
|
||||
if err != nil {
|
||||
return false, fmt.Errorf("failure updating PreExecutionError: %w", err)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -141,12 +141,13 @@ jobs:
|
|||
|
||||
func Test_tryHandleIncompleteMatrix(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
runJobID int64
|
||||
errContains string
|
||||
consumed bool
|
||||
runJobNames []string
|
||||
preExecutionError string
|
||||
name string
|
||||
runJobID int64
|
||||
errContains string
|
||||
consumed bool
|
||||
runJobNames []string
|
||||
preExecutionError actions_model.PreExecutionError
|
||||
preExecutionErrorDetails []any
|
||||
}{
|
||||
{
|
||||
name: "not incomplete_matrix",
|
||||
|
|
@ -164,9 +165,10 @@ func Test_tryHandleIncompleteMatrix(t *testing.T) {
|
|||
errContains: "jobStatusResolver attempted to tryHandleIncompleteMatrix for a job (id=603) with an incomplete 'needs' job (id=604)",
|
||||
},
|
||||
{
|
||||
name: "missing needs for strategy.matrix evaluation",
|
||||
runJobID: 605,
|
||||
preExecutionError: "Unable to evaluate `strategy.matrix` of job job_1 due to a `needs` expression that was invalid. It may reference a job that is not in it's 'needs' list (define-matrix-1), or an output that doesn't exist on one of those jobs.",
|
||||
name: "missing needs for strategy.matrix evaluation",
|
||||
runJobID: 605,
|
||||
preExecutionError: actions_model.ErrorCodePersistentIncompleteMatrix,
|
||||
preExecutionErrorDetails: []any{"job_1", "define-matrix-1"},
|
||||
},
|
||||
{
|
||||
name: "matrix expanded to 0 jobs",
|
||||
|
|
@ -258,10 +260,12 @@ func Test_tryHandleIncompleteMatrix(t *testing.T) {
|
|||
}
|
||||
slices.Sort(allJobNames)
|
||||
assert.Equal(t, tt.runJobNames, allJobNames)
|
||||
} else if tt.preExecutionError != "" {
|
||||
} else if tt.preExecutionError != 0 {
|
||||
// expectations are that the ActionRun has a populated PreExecutionError, is marked as failed
|
||||
actionRun := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: blockedJob.RunID})
|
||||
assert.Equal(t, tt.preExecutionError, actionRun.PreExecutionError)
|
||||
assert.Empty(t, actionRun.PreExecutionError)
|
||||
assert.Equal(t, tt.preExecutionError, actionRun.PreExecutionErrorCode)
|
||||
assert.Equal(t, tt.preExecutionErrorDetails, actionRun.PreExecutionErrorDetails)
|
||||
assert.Equal(t, actions_model.StatusFailure, actionRun.Status)
|
||||
|
||||
// ActionRunJob is marked as failed
|
||||
|
|
|
|||
|
|
@ -26,7 +26,6 @@ import (
|
|||
"forgejo.org/modules/log"
|
||||
"forgejo.org/modules/setting"
|
||||
api "forgejo.org/modules/structs"
|
||||
"forgejo.org/modules/translation"
|
||||
"forgejo.org/modules/util"
|
||||
webhook_module "forgejo.org/modules/webhook"
|
||||
"forgejo.org/services/convert"
|
||||
|
|
@ -405,8 +404,8 @@ func handleWorkflows(
|
|||
|
||||
var jobs []*jobparser.SingleWorkflow
|
||||
if dwf.EventDetectionError != nil { // don't even bother trying to parse jobs due to event detection error
|
||||
tr := translation.NewLocale(input.Doer.Language)
|
||||
run.PreExecutionError = tr.TrString("actions.workflow.event_detection_error", dwf.EventDetectionError)
|
||||
run.PreExecutionErrorCode = actions_model.ErrorCodeEventDetectionError
|
||||
run.PreExecutionErrorDetails = []any{dwf.EventDetectionError.Error()}
|
||||
run.Status = actions_model.StatusFailure
|
||||
jobs = []*jobparser.SingleWorkflow{{
|
||||
Name: dwf.EntryName,
|
||||
|
|
@ -420,8 +419,8 @@ func handleWorkflows(
|
|||
)
|
||||
if err != nil {
|
||||
log.Info("jobparser.Parse: invalid workflow, setting job status to failed: %v", err)
|
||||
tr := translation.NewLocale(input.Doer.Language)
|
||||
run.PreExecutionError = tr.TrString("actions.workflow.job_parsing_error", err)
|
||||
run.PreExecutionErrorCode = actions_model.ErrorCodeJobParsingError
|
||||
run.PreExecutionErrorDetails = []any{err.Error()}
|
||||
run.Status = actions_model.StatusFailure
|
||||
jobs = []*jobparser.SingleWorkflow{{
|
||||
Name: dwf.EntryName,
|
||||
|
|
|
|||
|
|
@ -216,7 +216,9 @@ func TestActionsNotifier_PreExecutionErrorInvalidJobs(t *testing.T) {
|
|||
createdRun := runs[0]
|
||||
|
||||
assert.Equal(t, actions_model.StatusFailure, createdRun.Status)
|
||||
assert.Contains(t, createdRun.PreExecutionError, "actions.workflow.job_parsing_error%!(EXTRA *fmt.wrapError=")
|
||||
assert.Empty(t, createdRun.PreExecutionError)
|
||||
assert.Equal(t, actions_model.ErrorCodeJobParsingError, createdRun.PreExecutionErrorCode)
|
||||
assert.Equal(t, []any{"model.ReadWorkflow: yaml: unmarshal errors:\n line 1: cannot unmarshal !!str `hello, ...` into map[string]*model.Job"}, createdRun.PreExecutionErrorDetails)
|
||||
}
|
||||
|
||||
func TestActionsNotifier_PreExecutionEventDetectionError(t *testing.T) {
|
||||
|
|
@ -239,7 +241,9 @@ func TestActionsNotifier_PreExecutionEventDetectionError(t *testing.T) {
|
|||
createdRun := runs[0]
|
||||
|
||||
assert.Equal(t, actions_model.StatusFailure, createdRun.Status)
|
||||
assert.Equal(t, "actions.workflow.event_detection_error%!(EXTRA *errors.errorString=nothing is not a valid event)", createdRun.PreExecutionError)
|
||||
assert.Empty(t, createdRun.PreExecutionError)
|
||||
assert.Equal(t, actions_model.ErrorCodeEventDetectionError, createdRun.PreExecutionErrorCode)
|
||||
assert.Equal(t, []any{"nothing is not a valid event"}, createdRun.PreExecutionErrorDetails)
|
||||
}
|
||||
|
||||
func TestActionsNotifier_handleWorkflows_setRunTrustForPullRequest(t *testing.T) {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue