Skip to content

Clean up Task error handling. #839

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 30, 2025

Conversation

Lukasa
Copy link
Collaborator

@Lukasa Lukasa commented Apr 30, 2025

Motivation

We have some Task error handling functions that are generic for no apparent reason. They're also typically called from contexts where they also report the error to the delegate, but one of the call sites doesn't do that. So add a test for that as well.

Modifications

  • Rewrite Task.fail(with:delegate:) to be non-generic.
  • Add a call to the delegate error handler on the path that is missing it.
  • Add a test for that call

Results

Cleaner, easier to follow code

Motivation

We have some Task error handling functions that are generic for
no apparent reason. They're also typically called from contexts
where they also report the error to the delegate, but one of the
call sites doesn't do that. So add a test for that as well.

Modifications

- Rewrite Task.fail(with:delegate:) to be non-generic.
- Add a call to the delegate error handler on the path that is
    missing it.
- Add a test for that call

Results

Cleaner, easier to follow code
@Lukasa Lukasa added the 🔨 semver/patch No public API change. label Apr 30, 2025
@Lukasa Lukasa enabled auto-merge (squash) April 30, 2025 13:51
@Lukasa Lukasa merged commit beb2637 into swift-server:main Apr 30, 2025
23 of 24 checks passed
@Lukasa Lukasa deleted the cb-delete-weird-unnecessary-code branch April 30, 2025 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants