Surface why a group or project transfer failed

What does this MR do and why?

Fixes Group/Project Namespace Transfers Fail Silently (#597297 - closed) • Rémy Coutable • 19.5 (severity::1, priority::1, SLO::Missed, two customer reports).

Epic Make group/project transfer async with the `tra... (&20521 - closed) • Shubham Kumar made project and group transfers asynchronous and added a to-do item on failure (#599819+s). What was missing was the reason. The to-do item read Failed to transfer X to Y. Try again. with no explanation, and telling a user to "Try again" on an unfixable validation error just re-queued the same doomed transfer.

This MR makes four changes.

1. Persist and surface the failure reason

Both async transfer workers (Projects::TransferWorker, Namespaces::Groups::TransferWorker) called TransferService#execute, got back a bare false, and created a content-free to-do item. The real translated message (for example Project with same name or path in target namespace already exists) only reached Gitlab::AppLogger, which is admin-only.

The workers now pass the service's message into cancel_transfer!(transfer_error:). A new set_transfer_error_data state-machine callback records it in state_metadata['transfer_last_error'] — a key that was already reserved in TRANSFER_METADATA_KEYS, validated by the namespace_detail_state_metadata JSON schema, and exposed as a jsonb_accessor on Namespace::Detail, but written by nothing until now. This mirrors the existing deletion flow (set_deletion_error_data / deletion_error).

The to-do item renders the reason via a new Todo#transfer_failure_reason, a new transferFailureReason GraphQL field, and a second i18n string. Unexpected exceptions (anything that is not a TransferError) get a generic user-facing message instead, so raw error text — which can contain SQL fragments or paths — stays in the logs only.

Two things worth a reviewer's attention:

  • The new callback is declared after the clear_transfer_data / clear_transfer_data_preserving_target callbacks, because state_machine runs before_transition hooks in declaration order and those two callbacks clear transfer_last_error. There are comments at both sites explaining the ordering.
  • The error message is truncated to 500 characters, since state_metadata has a 64 KB schema size limit.

2. Fail fast on the project name/path conflict

Projects::TransferService#ensure_allowed_transfer — the schedule-time gate — only checked for a blank namespace, transferring to the same namespace, and permissions. The name/path conflict check lived inside #transfer, so it only ran in the worker: the user was told the transfer was scheduled, then the job died in Sidekiq. That is exactly the reproduction in the issue.

The check now moves into ensure_allowed_transfer so it fails inline at request time, on every entry point (controller flash, GraphQL errors, REST 400). The copy in #transfer is removed: #transfer only runs from #execute, right after ensure_allowed_transfer, so it could never trigger. The gap between scheduling and running is still covered, because Projects::TransferWorker calls #execute, which runs ensure_allowed_transfer again when the job starts. Groups already validated namespace_with_same_path? up front, so this change is project-only.

3. Correct the overpromising copy

The in-app banner, both list toasts, the ServiceResponse "queued" messages, and both docs pages promised an email on success or failure, and notifications for users with the Owner or Maintainer role. On failure there is no email at all, and only the user who started the transfer gets a to-do item. The copy now describes what actually happens.

4. Drop the redundant prefix on group transfer errors

Groups::TransferService wrapped every failure in TransferGroup|Transfer failed: %{error_message}. Once the reason reached the to-do item that stuttered: Failed to transfer X to Y: Transfer failed: Group is already a root group. The wrapper now applies only to ActiveRecord::RecordInvalid and Gitlab::UpdatePathError, which carry raw lowercase fragments such as namespace directory cannot be moved and do need framing. TransferError messages are curated full sentences and now stand on their own. This also makes the service consistent with Projects::TransferService and with its own Service Desk branch, neither of which prefixes.

Rendering the reason

The reason is stripped of any trailing ., !, or ? before interpolation, so the i18n string is the sole source of the final punctuation. Most reasons are already complete sentences, so appending a period unconditionally produced a double period on those paths.

The values are also passed to sprintf with escapeParameters set to false. gl-sprintf renders its non-placeholder text as plain Vue text nodes, and text nodes never decode HTML entities, so the default escaping would surface apostrophes as a literal ' — You don't have permission to transfer this project. Passing them unescaped is safe precisely because gl-sprintf never interprets the string as HTML; ci/pipeline_editor/components/header/validation_segment.vue does the same for the same reason.

Drive-by fix

Projects::TransferService declared attr_reader :error but never assigned @error, so service.error was always nil, unlike Groups::TransferService. It is assigned now; the reason-surfacing work in this MR depends on it.

Out of scope

No failure email is added — that's deliberately out of scope and worth a separate issue.

There is no new feature flag. groups_and_projects_async_transfer is already rolled out to everyone, and changes 2, 3, and 4 also apply to the synchronous (non-flagged) path.

Multiversion compatibility

Danger flags this MR because it changes both the GraphQL schema and the frontend. During a rolling deployment a canary frontend can hit a backend that does not yet know transferFailureReason, and an unknown field fails validation for the whole query rather than returning null. The field is therefore requested with @gl_introduced(version: "19.4.0") in todo.fragment.graphql, so an older backend skips it and the to-do falls back to the existing transferFailedMessage string.

References

The investigation write-up is posted as a comment on #597297 (closed).

Screenshots or screen recordings

Screenshots of the to-do item are still to be captured. In the meantime, the exact rendered strings, as asserted in spec/frontend/todos/components/todo_item_body_spec.js:

To-do item text
Before Failed to transfer gitlab-org-source to gitlab-org / my-source-group. Try again.
After (worker-time failure) Failed to transfer gitlab-org-source to gitlab-org / my-source-group: Namespace already taken. Try again.
After (unexpected exception) Failed to transfer gitlab-org-source to gitlab-org / my-source-group: The transfer failed unexpectedly. Contact your administrator if this keeps happening. Try again.
After (no reason recorded) Failed to transfer gitlab-org-source to gitlab-org / my-source-group. Try again.

How to set up and validate locally

Scenario A — the reported reproduction (now fails fast):

  1. Create a project test-project at the root namespace, a subgroup test-group, and a project test-group/test-project.
  2. In rails c, run Feature.enable(:groups_and_projects_async_transfer).
  3. Go to the root test-project > Settings > General > Advanced > Transfer project, pick test-group, and confirm.
  4. Expect an inline alert naming the conflict, and no Projects::TransferWorker job enqueued. Before this MR you got a "scheduled" banner and silence.

Scenario B — a worker-time failure (shows the reason in the to-do item):

  1. Force a failure the schedule-time gate cannot see — for example, stub Projects::TransferService#proceed_to_transfer to raise Projects::TransferService::TransferError, 'Something specific went wrong'.
  2. Start a transfer and let Sidekiq run it.
  3. Visit /dashboard/todos. Expect Failed to transfer … : Something specific went wrong. Try again.
  4. Confirm the persisted value: project.project_namespace.reload.transfer_last_error.

Tests

  • 437 backend examples, 0 failures, across spec/models/concerns/namespaces/stateful/transition_callbacks_spec.rb, spec/models/todo_spec.rb, spec/models/namespace/detail_spec.rb, spec/services/projects/transfer_service_spec.rb, spec/services/groups/transfer_service_spec.rb, spec/workers/projects/transfer_worker_spec.rb, spec/workers/namespaces/groups/transfer_worker_spec.rb, spec/graphql/types/todo_type_spec.rb, spec/helpers/namespaces/transfer_helper_spec.rb.
  • 45 Jest tests pass in spec/frontend/todos/components/todo_item_body_spec.js, including four new ones. The two covering punctuation and HTML-entity escaping were verified to fail without the fix in todo_item_body.vue.
  • To be transparent: the :js feature specs (spec/features/projects/settings/user_transfers_a_project_spec.rb, spec/features/groups/settings/advanced_spec.rb) and the two list-utils Jest specs could not be run locally for environment reasons, so CI is their first real run.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Shubham Kumar

Merge request reports

Loading
Loading