Skip to content

fix(backups-create): returns an error when the backup fails - #1266

Merged
EtienneM merged 5 commits into
Scalingo:masterfrom
josephpage:fix/backups-create-error
Oct 13, 2026
Merged

EtienneM merged 5 commits into
Scalingo:masterfrom
josephpage:fix/backups-create-error

Conversation

@josephpage

Copy link
Copy Markdown
Contributor

Context

backups-create waits for the backup to finish (it polls the backup status until it is done or error) and prints "Backup failed" on stderr when the backup ends in error, but it returns no error:

	if backup.Status == scalingo.BackupStatusDone {
		io.Status(color.New(color.FgGreen).Sprint("Backup successfully finished"))
	} else {
		io.Error(color.New(color.FgRed).Sprintf("Backup failed"))
	}
	return nil

So the command exits with the status 0 even when the backup failed. In a CI/CD pipeline (the typical use case of a manual backup: back up before deploying a new version), the next step is run anyway.

Other commands do fail

Returning an error when the asynchronous operation they wait for fails is what other commands do:

  • deployments.Stream (used by deploy and deployment-follow) returns ErrDeploymentFailed:
				if !anyDeployment && scalingo.IsFinishedString(scalingo.DeploymentStatus(statusData.Content)) {
					if scalingo.HasFailedString(scalingo.DeploymentStatus(statusData.Content)) {
						return ErrDeploymentFailed
					}
					return nil
				}
  • apps.OperationWaiter.WaitOperation (used by restart, scale and run) returns an error:
			case "error":
				fmt.Printf("\bOperation '%s' failed, an error occurred: %v\n", op.Type, op.Error)
				return op, errors.Newf(ctx, "operation %v failed", op.ID)

In both cases the cmd layer calls errorQuit(ctx, err), which exits with the code 1.

Changes

db.CreateBackup returns an error when the backup does not end in the done status, so backups-create now exits with the code 1. The failure is displayed by the standard error reporting (An error occurred: backup <id> failed) instead of the custom message.

  • Add a changelog entry in the section "To Be Released" of CHANGELOG.md

Notes

  • this is a behaviour change, and that is the point: a script relying on the previous behaviour (a failed backup not failing the command) now has to handle the error. Tell me if you would rather keep it for a major version.
  • gofmt reports nothing on the modified file. I did not add a test: the repository tests pure helpers, and this path would need a mocked client (config.ScalingoClient is a package-level function). Happy to add one if you have a pattern in mind.

@josephpage
josephpage force-pushed the fix/backups-create-error branch from bc71677 to e11c476 Compare October 12, 2026 12:49
@EtienneM
EtienneM self-requested a review October 13, 2026 08:19
@EtienneM EtienneM changed the title fix(backups): fail when the backup ends in error fix(backups-create): returns an error when the backup fails Oct 13, 2026

@EtienneM EtienneM left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution

josephpage and others added 5 commits October 13, 2026 17:15
`backups-create` waits for the backup to finish and reports "Backup failed" on
stderr when it ends in error, but it returns no error: the command exits with
the status 0, and a CI/CD pipeline runs the next step even though the backup
failed.

Commands waiting for an asynchronous operation do return an error:
`deployments.Stream` returns `ErrDeploymentFailed` when a deployment fails, and
`apps.OperationWaiter.WaitOperation` returns an error when the operation fails,
both leading to `errorQuit` and an exit code of 1.

The failure is now reported by the standard error handling
(`An error occurred: backup <id> failed`) instead of a custom message.
@EtienneM
EtienneM force-pushed the fix/backups-create-error branch from 74b8ce8 to 40d1b7a Compare October 13, 2026 15:16
@EtienneM
EtienneM enabled auto-merge October 13, 2026 15:16
@EtienneM
EtienneM merged commit 4ff3f78 into Scalingo:master Oct 13, 2026
7 checks passed
@josephpage
josephpage deleted the fix/backups-create-error branch October 13, 2026 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants