Skip to content
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

Throw exception when the CancellationToken is cancelled as soon as possible #70

Closed
akamsteeg opened this issue Jul 16, 2022 · 0 comments
Assignees
Labels
BREAKING CHANGE Breaking change that involves increasing the major version number enhancement New feature or request good first issue Good for newcomers
Milestone

Comments

@akamsteeg
Copy link
Owner

akamsteeg commented Jul 16, 2022

In V6 or earlier, we rely on framework code (mainly System.Text.Json or the HttpClient) to throw an exception when a supplied CancellationToken is cancelled. I think it's good form to cancel as early as possible, so we should add a cancellationToken.ThrowIfCancellationRequested() to our HaveIBeenPwnedClient.*Internal() methods after any argument and disposal checks.

⚠️ Warning: This might break user code that catches TaskCanceledException instead of OperationCanceledException. I have no idea if the HttpClient or System.Text.Json have used TaskCanceledException in the past.

@akamsteeg akamsteeg added enhancement New feature or request help wanted Extra attention is needed good first issue Good for newcomers labels Jul 16, 2022
@akamsteeg akamsteeg added this to the 6.x milestone Jul 16, 2022
@akamsteeg akamsteeg added the BREAKING CHANGE Breaking change that involves increasing the major version number label Jul 16, 2022
@akamsteeg akamsteeg modified the milestones: 6.x, 7.x Jul 16, 2022
akamsteeg added a commit that referenced this issue Jul 16, 2022
@akamsteeg akamsteeg removed the help wanted Extra attention is needed label Jul 31, 2022
@akamsteeg akamsteeg self-assigned this Nov 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BREAKING CHANGE Breaking change that involves increasing the major version number enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

1 participant