Use ServiceResponse in services
Problem
Contributes to &2294.
Currently, our services tend to return different response types:
- Ancient services return an
object
(e.g.issue
ormerge_request
) wherevalid?
/errors
are checked ornil
- Many services return a response hash (see
BaseService
) - New services return
ServiceResponse
(added in gitlab-foss!27516 (merged)) as described in our Guidelines for reusing abstractions.
For consistency, services should only have a single response type: ServiceResponse
.
Proposed Solution
Return SerivceResponse
objects from services universally and consistently.
Proposed Implementation
- Collect services' response types per namespaces (if namespaced).
- Adjust services per namespace.
Example: ErrorTracking
uses the "response hash" so changing to ServiceResponse
should be done in all services of this namespace.
Problems
Despite the effort of actually changing the response types in services we have to find a pragmatic solution for error handling and or deferring executing when chaining service responses - meaning: Railway Oriented Programming (or short ROP).
Example: Error handling
For error handling we already using guard clauses + ServiceResponse.error(...)
Example MergeRequests::MergeabilityCheckService
:
def execute(recheck: false, retry_lease: true)
return service_error if service_error
return check_mergeability(recheck) unless merge_ref_auto_sync_lock_enabled?
...
end
It's unclear how to improve this approach further.
Example: Deferring execution
Issues::UpdateService#execute
:
def execute(issue)
...
move_issue_to_new_project(issue) || update_task_event(issue) || update(issue)
end
Here, nil
is used to abort and defer the execution to the next task. This won't work with ServiceResponse
.
We need to find a way to incorporate this approach (deferring execution) into ServiceResponse
or reconsider the implementation of these services.
Response hash and error handling
For "response hashes" we use Stepable
in some services to streamline error handling. E.g: Metrics::Dashboard::UpdateDashboardService
. This approach is not perfect and has its own problems e.g. #207391 but it works