Convert async -> Result signature functions to async throws

[toupper]

Closes: #8104 (Sorry for the bigger PR)

Description

With this PR I convert the function signatures with await -> Result<..., Error> to the most natural async throws -> ..., except in those cases where we have a specific error type e.g here

Testing instructions

Unit tests should pass. You can also check that the related features do not present any side effects:

• Remove Apple Id access
• Create account
• Domain suggestions
• Username suggestions
• Free domain suggestions
• Site creation
• Just in Time Messages
• Stats

Screenshots

---

• I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

[koke]

I was going to suggest that we could replace this chunk of code using the existing helpers to convert a throwing call to a Result, and a Result to a continuation. After trying it, the code is similarly long and I believe less readable, so the current approach feels better 🤷🏽‍♂️

Leaving it here in case it helps for other future implementations:

continuation.resume(
    with: result.flatMap { data in
        Result {
            let validator = request.responseDataValidator()
            try validator.validate(data: data)
            return try mapper.map(response: data)
        }
    }.mapError({ error in
        DDLogError("<> Mapping Error: \(error)")
        self.handleResponseError(error: error, for: request)
        return error
    })
)

[jaclync]

Thanks for tagging me in this one! I see that this PR is going to conflict with another PR of mine #8154 which is ready to merge but is blocked by CI, I just disabled auto-merge in case of bad timing. I'm also working on another task that will conflict with the part about removing Apple ID access #8178, I will follow this PR and rebase when needed.

Updated: the second PR with potential merge conflicts is #8179

[koke]

Made a few suggestions to simplify a bit, but overall it looks good. When I started reviewing I saw unit tests failing, but whenever those are fixed I'd say it's good.

[koke]

Perhaps we can use try? here to simplify this block if we want to ignore the error case

[toupper]

Sure, done in 5b2efe2

[koke]

I was going to suggest Result(catching:) here but then realized it's async. In the original conversation, I suggested an extension to make an async catching initializer:

extension Result where Failure == Error {
    init(catching body: () async throws -> Success) async {
        do {
            let value = try await body()
            self = .success(value)
        } catch {
            self = .failure(error)
        }
    }
}

With that, this could be simplified as:

private extension DomainStore {
    func loadFreeDomainSuggestions(query: String, completion: @escaping (Result<[FreeDomainSuggestion], Error>) -> Void) {
        Task { @MainActor in
            let result = await Result {
                try await remote.loadFreeDomainSuggestions(query: query)
            }
            completion(result)
        }
    }
}

I see this also apply to several other diffs in Yosemite, so it might make sense to add it on this PR

[toupper]

Nice improvement! Added in b00cd6a

[koke]

Result.get() is a convenient way to convert from result to throwing:

Suggested change

	switch result {
	case let .success(suggestions):
	return suggestions
	case let .failure(error):
	throw error
	return try result.get()

I see a couple other instances of this in YosemiteTests

[toupper]

True, done in 1a62a27

[peril-woocommerce]

	Warnings
⚠️	PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS

[toupper]

@jaclync Thanks for letting me know about these possible conflicts! Do you want me to wait until your PRs are merged before merging this one?

[jaclync]

Do you want me to wait until your PRs are merged before merging this one?

It depends on when the CI is fixed 😅 I was hoping to merge #8154 ASAP since multiple branches (one PR also ready to merge) depend on it and I'll resolve the merge conflicts in your branch here, but the CI is still not working ATM and I don't know if will by the EOD my time today.

To unblock other work that depends on #8154 and your branch, I'm going to merge your branch to #8154 branch locally and resolve merge conflicts. If the CI is fixed during my day, I'll merge #8154 and then resolve merge conflicts in a commit based on your branch (you can decide whether to use it). Otherwise, feel free to merge your PR if CI is ready during your day and I'll resolve the conflicts tomorrow 🙂

[jaclync]

Update: actually, there are no merge conflicts with the main PR I was hoping to merge! I probably misread the changes earlier. Another PR #8179 is likely having merge conflicts but this is at a lower priority and I can handle them later. Please go ahead with merging this anytime! 😄

[wpmobilebot]

You can test the changes from this Pull Request by:

• Clicking here or scanning the QR code below to access App Center
• Then installing the build number pr8176-b00cd6a on your iPhone

If you need access to App Center, please ask a maintainer to add you.

[toupper]

Great, thanks for the update @jaclync !

[joshheald]

Thanks for tagging me in this. I don't have any comments to add to Koke's and Jaclyn's, but the code looks good to me and tests well. I always appreciate learning from a PR!

[mokagio]

💜