Keep batch notification failures out of report counts
This commit is contained in:
@@ -154,13 +154,14 @@ func (e BatchError) Error() string {
|
|||||||
if e.Result == nil {
|
if e.Result == nil {
|
||||||
return "batch failed"
|
return "batch failed"
|
||||||
}
|
}
|
||||||
if batchNotificationFailed(e.Result) && batchReportFailures(e.Result) == 0 {
|
failedReports := batchReportFailures(e.Result)
|
||||||
|
if batchNotificationFailed(e.Result) && failedReports == 0 {
|
||||||
if e.Result.Notification.Error != "" {
|
if e.Result.Notification.Error != "" {
|
||||||
return fmt.Sprintf("batch %s notification failed: %s", e.Result.Batch, e.Result.Notification.Error)
|
return fmt.Sprintf("batch %s notification failed: %s", e.Result.Batch, e.Result.Notification.Error)
|
||||||
}
|
}
|
||||||
return fmt.Sprintf("batch %s notification failed", e.Result.Batch)
|
return fmt.Sprintf("batch %s notification failed", e.Result.Batch)
|
||||||
}
|
}
|
||||||
return fmt.Sprintf("batch %s failed: %d of %d reports failed", e.Result.Batch, e.Result.Failed, e.Result.Total)
|
return fmt.Sprintf("batch %s failed: %d of %d reports failed", e.Result.Batch, failedReports, len(e.Result.Reports))
|
||||||
}
|
}
|
||||||
|
|
||||||
func batchNotificationFailed(result *BatchResult) bool {
|
func batchNotificationFailed(result *BatchResult) bool {
|
||||||
@@ -291,7 +292,7 @@ func RunBatch(ctx context.Context, req BatchRequest) error {
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
if result.Failed > 0 {
|
if result.Failed > 0 || batchNotificationFailed(result) {
|
||||||
return BatchError{Result: result}
|
return BatchError{Result: result}
|
||||||
}
|
}
|
||||||
return nil
|
return nil
|
||||||
@@ -370,13 +371,10 @@ func RunBatchDetailed(ctx context.Context, req BatchRequest) (*BatchResult, erro
|
|||||||
result.Reports = append(result.Reports, item)
|
result.Reports = append(result.Reports, item)
|
||||||
}
|
}
|
||||||
result.Total = len(result.Reports)
|
result.Total = len(result.Reports)
|
||||||
batchNotification, err := notifyBatch(ctx, req.Config, req.Batch, batchRunID(startedAt, req.Batch), startedAt, result, plannedReports, req.Notifier)
|
batchNotification, _ := notifyBatch(ctx, req.Config, req.Batch, batchRunID(startedAt, req.Batch), startedAt, result, plannedReports, req.Notifier)
|
||||||
if batchNotification != nil {
|
if batchNotification != nil {
|
||||||
result.Notification = batchNotification
|
result.Notification = batchNotification
|
||||||
}
|
}
|
||||||
if err != nil {
|
|
||||||
result.Failed++
|
|
||||||
}
|
|
||||||
result.FinishedAt = time.Now()
|
result.FinishedAt = time.Now()
|
||||||
return result, nil
|
return result, nil
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -2,8 +2,10 @@ package app
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"errors"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
"gitea.maximumdirect.net/eric/weatherreporter/internal/config"
|
"gitea.maximumdirect.net/eric/weatherreporter/internal/config"
|
||||||
@@ -95,6 +97,43 @@ func TestRunBatchDetailedPreflightsAllOutputPaths(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestRunBatchDetailedRetainsReportCountsWhenNotificationFails(t *testing.T) {
|
||||||
|
bundle := generationBundle(t)
|
||||||
|
bundle.Hourly.Periods = bundle.Hourly.Periods[:1]
|
||||||
|
outputDir := t.TempDir()
|
||||||
|
notifier := &generationNotifier{batchErr: errors.New("distributor unavailable")}
|
||||||
|
result, err := RunBatchDetailed(context.Background(), BatchRequest{
|
||||||
|
Config: generationDistributorConfig(), Batch: BatchMorning,
|
||||||
|
Now: generationTime("2026-05-29T08:30:00-05:00"), WorkingDir: t.TempDir(), OutputDir: outputDir,
|
||||||
|
Collector: &generationCollector{bundle: &bundle}, Executor: &generationExecutor{}, Notifier: notifier,
|
||||||
|
})
|
||||||
|
if err != nil || result == nil || result.Total != len(result.Reports) || result.Succeeded != len(result.Reports) || result.Failed != 0 || result.Notification == nil || result.Notification.Status != "failed" {
|
||||||
|
t.Fatalf("RunBatchDetailed() result/error = %#v/%v", result, err)
|
||||||
|
}
|
||||||
|
for _, item := range result.Reports {
|
||||||
|
if item.Status != "succeeded" || item.OutputPath == "" {
|
||||||
|
t.Fatalf("report result = %#v", item)
|
||||||
|
}
|
||||||
|
if _, statErr := os.Stat(item.OutputPath); statErr != nil {
|
||||||
|
t.Fatalf("published output %q: %v", item.OutputPath, statErr)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestRunBatchReturnsNotificationFailureWithoutReportFailureWording(t *testing.T) {
|
||||||
|
bundle := generationBundle(t)
|
||||||
|
bundle.Hourly.Periods = bundle.Hourly.Periods[:1]
|
||||||
|
err := RunBatch(context.Background(), BatchRequest{
|
||||||
|
Config: generationDistributorConfig(), Batch: BatchMorning,
|
||||||
|
Now: generationTime("2026-05-29T08:30:00-05:00"), WorkingDir: t.TempDir(), OutputDir: t.TempDir(),
|
||||||
|
Collector: &generationCollector{bundle: &bundle}, Executor: &generationExecutor{}, Notifier: &generationNotifier{batchErr: errors.New("distributor unavailable")},
|
||||||
|
})
|
||||||
|
var batchErr BatchError
|
||||||
|
if !errors.As(err, &batchErr) || batchErr.Result == nil || batchErr.Result.Failed != 0 || batchErr.Result.Notification == nil || batchErr.Result.Notification.Status != "failed" || !strings.Contains(err.Error(), "notification failed") || strings.Contains(err.Error(), "reports failed") {
|
||||||
|
t.Fatalf("RunBatch() error/result = %v/%#v", err, batchErr.Result)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func generationDistributorConfig() config.Config {
|
func generationDistributorConfig() config.Config {
|
||||||
cfg := generationConfig()
|
cfg := generationConfig()
|
||||||
cfg.Notify.Distributor.Enabled = true
|
cfg.Notify.Distributor.Enabled = true
|
||||||
|
|||||||
@@ -243,6 +243,7 @@ func generationBundlePointer(t *testing.T) *weatherdata.Bundle {
|
|||||||
|
|
||||||
type generationNotifier struct {
|
type generationNotifier struct {
|
||||||
err error
|
err error
|
||||||
|
batchErr error
|
||||||
request NotificationRequest
|
request NotificationRequest
|
||||||
batchRequest batchNotificationRequest
|
batchRequest batchNotificationRequest
|
||||||
batchCalls int
|
batchCalls int
|
||||||
@@ -261,7 +262,7 @@ func (n *generationNotifier) NotifyBatch(_ context.Context, request batchNotific
|
|||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
return &NotificationResult{Status: "succeeded", PipelineID: request.PipelineID, BundleID: request.BundleID}, nil
|
return &NotificationResult{Status: "succeeded", PipelineID: request.PipelineID, BundleID: request.BundleID}, n.batchErr
|
||||||
}
|
}
|
||||||
|
|
||||||
func generationBundle(t *testing.T) weatherdata.Bundle {
|
func generationBundle(t *testing.T) weatherdata.Bundle {
|
||||||
|
|||||||
@@ -40,10 +40,11 @@ Options:
|
|||||||
`
|
`
|
||||||
|
|
||||||
type Runner struct {
|
type Runner struct {
|
||||||
Clock timeutil.Clock
|
Clock timeutil.Clock
|
||||||
ExecutorFactory ExecutorFactory
|
ExecutorFactory ExecutorFactory
|
||||||
Version string
|
Version string
|
||||||
WorkingDir string
|
WorkingDir string
|
||||||
|
runBatchDetailed func(context.Context, app.BatchRequest) (*app.BatchResult, error)
|
||||||
}
|
}
|
||||||
|
|
||||||
func Run(ctx context.Context, args []string, stdout io.Writer, stderr io.Writer) error {
|
func Run(ctx context.Context, args []string, stdout io.Writer, stderr io.Writer) error {
|
||||||
@@ -89,7 +90,11 @@ func (r Runner) Run(ctx context.Context, args []string, stdout io.Writer, stderr
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
result, err := app.RunBatchDetailed(ctx, req)
|
runBatchDetailed := r.runBatchDetailed
|
||||||
|
if runBatchDetailed == nil {
|
||||||
|
runBatchDetailed = app.RunBatchDetailed
|
||||||
|
}
|
||||||
|
result, err := runBatchDetailed(ctx, req)
|
||||||
if result != nil {
|
if result != nil {
|
||||||
summary := newBatchSummary(result)
|
summary := newBatchSummary(result)
|
||||||
if encodeErr := writeActionResult(stdout, stderr, summary, outputOptions{Quiet: opts.Quiet}, func(w io.Writer) {
|
if encodeErr := writeActionResult(stdout, stderr, summary, outputOptions{Quiet: opts.Quiet}, func(w io.Writer) {
|
||||||
|
|||||||
@@ -3,12 +3,15 @@ package cli
|
|||||||
import (
|
import (
|
||||||
"bytes"
|
"bytes"
|
||||||
"context"
|
"context"
|
||||||
|
"encoding/json"
|
||||||
|
"errors"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
|
"gitea.maximumdirect.net/eric/weatherreporter/internal/app"
|
||||||
"gitea.maximumdirect.net/eric/weatherreporter/internal/promptexec"
|
"gitea.maximumdirect.net/eric/weatherreporter/internal/promptexec"
|
||||||
"gitea.maximumdirect.net/eric/weatherreporter/internal/timeutil"
|
"gitea.maximumdirect.net/eric/weatherreporter/internal/timeutil"
|
||||||
)
|
)
|
||||||
@@ -80,6 +83,43 @@ func TestResolveGenerateActionUsesInjectedWorkingDirectoryForOutputOverrides(t *
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestRunActionReturnsFailureForBatchNotificationFailure(t *testing.T) {
|
||||||
|
configPath := filepath.Join(t.TempDir(), "config.yml")
|
||||||
|
if err := os.WriteFile(configPath, []byte("weather_api:\n base_url: https://weather.api.example.com/\n"), 0o600); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
result := &app.BatchResult{
|
||||||
|
Batch: app.BatchMorning, Total: 2, Succeeded: 2,
|
||||||
|
Reports: []app.BatchReportResult{
|
||||||
|
{ReportID: "today", Status: "succeeded", OutputPath: "/reports/today.md"},
|
||||||
|
{ReportID: "tomorrow", Status: "succeeded", OutputPath: "/reports/tomorrow.md"},
|
||||||
|
},
|
||||||
|
Notification: &app.BatchNotificationResult{Status: "failed", Error: "distributor unavailable"},
|
||||||
|
}
|
||||||
|
var stdout, stderr bytes.Buffer
|
||||||
|
runner := Runner{
|
||||||
|
Clock: timeutil.FixedClock{Time: time.Date(2026, 5, 29, 8, 0, 0, 0, time.UTC)},
|
||||||
|
ExecutorFactory: func(PromptExecutorConfig) (promptexec.Executor, error) {
|
||||||
|
return &factoryExecutor{}, nil
|
||||||
|
},
|
||||||
|
runBatchDetailed: func(context.Context, app.BatchRequest) (*app.BatchResult, error) {
|
||||||
|
return result, nil
|
||||||
|
},
|
||||||
|
}
|
||||||
|
err := runner.Run(context.Background(), []string{"run", "morning", "--config", configPath}, &stdout, &stderr)
|
||||||
|
var batchErr app.BatchError
|
||||||
|
if !errors.As(err, &batchErr) || !strings.Contains(err.Error(), "notification failed") {
|
||||||
|
t.Fatalf("Run() error = %v", err)
|
||||||
|
}
|
||||||
|
var summary batchSummary
|
||||||
|
if err := json.Unmarshal(stdout.Bytes(), &summary); err != nil {
|
||||||
|
t.Fatalf("decode summary: %v", err)
|
||||||
|
}
|
||||||
|
if summary.Status != summaryStatusFailed || summary.Total != 2 || summary.Succeeded != 2 || summary.Failed != 0 || summary.Notification == nil || summary.Notification.Status != "failed" {
|
||||||
|
t.Fatalf("summary = %#v", summary)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestInspectCommandIsUnknownAndAbsentFromHelp(t *testing.T) {
|
func TestInspectCommandIsUnknownAndAbsentFromHelp(t *testing.T) {
|
||||||
var stdout, stderr bytes.Buffer
|
var stdout, stderr bytes.Buffer
|
||||||
err := (Runner{}).Run(context.Background(), []string{"inspect", "reports"}, &stdout, &stderr)
|
err := (Runner{}).Run(context.Background(), []string{"inspect", "reports"}, &stdout, &stderr)
|
||||||
|
|||||||
Reference in New Issue
Block a user