Allow treating unsuccessful status code as non-error, avoid setting exception

[alexeyzimarev]

User description

Description

Add an option to not set the response exception if the call went toruhg but status code is unsuccessful.

Fixes #2302

Purpose

This pull request is a:

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

---

PR Type

Enhancement

---

Description

• Add ErrorWhenUnsuccessfulStatusCode option to control exception behavior

• Modify MaybeException() to accept parameter for conditional error handling

• Refactor FromHttpResponse() to accept ReadOnlyRestClientOptions instead of individual parameters

• Add test coverage for unsuccessful status code without exception

---

Diagram Walkthrough

flowchart LR
  A["RestClientOptions"] -->|"ErrorWhenUnsuccessfulStatusCode"| B["MaybeException()"]
  B -->|"false"| C["No Exception Set"]
  B -->|"true"| D["Exception Set"]
  E["FromHttpResponse()"] -->|"uses Options"| B

Loading

File Walkthrough

	Relevant files
Enhancement	RestClientOptions.cs Add ErrorWhenUnsuccessfulStatusCode configuration option  src/RestSharp/Options/RestClientOptions.cs Add new ErrorWhenUnsuccessfulStatusCode property with default value true Property controls whether exceptions are thrown for unsuccessful HTTP status codes Includes XML documentation explaining the behavior +6/-0      HttpResponseExtensions.cs Add parameter to control exception creation behavior          src/RestSharp/Extensions/HttpResponseExtensions.cs Modify MaybeException() method to accept throwOnUnsuccessfulStatusCode parameter Update logic to return null when parameter is false regardless of status code Enables conditional exception creation based on client configuration +2/-2      RestResponse.cs Refactor to use options object in response creation            src/RestSharp/Response/RestResponse.cs Refactor FromHttpResponse() method signature to accept ReadOnlyRestClientOptions instead of individual parameters Replace encoding and calculateResponseStatus parameters with options parameter Update method calls to use options.Encoding and options.CalculateResponseStatus Pass options.ErrorWhenUnsuccessfulStatusCode to MaybeException() call Remove unused System.Text import +9/-11    RestClient.Async.cs Update method calls to use new options parameter                  src/RestSharp/RestClient.Async.cs Update ExecuteAsync() call to FromHttpResponse() with new signature Pass Options object instead of individual Encoding and CalculateResponseStatus parameters Update DownloadStreamAsync() to pass Options.ErrorWhenUnsuccessfulStatusCode to MaybeException() +2/-3
Formatting	ReadOnlyRestClientOptions.cs Remove commented code                                                                        src/RestSharp/Options/ReadOnlyRestClientOptions.cs Remove commented-out partial method declaration Clean up code formatting +0/-1
Tests	RequestFailureTests.cs Add test coverage for new option behavior                                test/RestSharp.Tests.Integrated/RequestFailureTests.cs Add assertions to verify error exception is set on unsuccessful status code Add new test Does_not_throw_on_unsuccessful_status_code_with_option() to verify exception is not set when option is disabled Add ReSharper comment to suppress warning about disposed closure +15/-0

---

[cloudflare-workers-and-pages]

Deploying restsharp with    Cloudflare Pages

Latest commit:	8a8f153
Status:	✅  Deploy successful!
Preview URL:	https://48bb00c7.restsharp.pages.dev
Branch Preview URL:	https://error-unsuccessful-status-co.restsharp.pages.dev

View logs

[qodo-merge-for-open-source]

PR Compliance Guide 🔍

(Compliance updated until commit 8a8f153)

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🟡	🎫 #2302 🟢 Add an option to RestClientOptions to control whether response.ErrorException should be populated when response.IsSuccessStatusCode is false Allow users to distinguish actual exceptions from status code failures without relying on message string parsing When the option is enabled, response.ErrorException should be null for unsuccessful status codes (only set for actual exceptions) 🔴 The option should be off by default to maintain existing behavior and avoid breaking changes
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

---

Previous compliance checks

Compliance check up to commit f8e891a

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Silent failure risk: When throwOnUnsuccessfulStatusCode is false, unsuccessful HTTP responses return null exception, potentially causing silent failures without proper error handling downstream. Referred Code public static Exception? MaybeException(this HttpResponseMessage httpResponse, bool throwOnUnsuccessfulStatusCode) => httpResponse.IsSuccessStatusCode || !throwOnUnsuccessfulStatusCode ? null Learn more about managing compliance generic rules or creating your own custom rules

[qodo-merge-for-open-source]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Inconsistent error handling behavior The new ErrorWhenUnsuccessfulStatusCode option leads to inconsistent error handling between ExecuteAsync and DownloadStreamAsync. Standardize the approach to ensure predictable behavior, as DownloadStreamAsync can obscure failures when the option is disabled. Examples: src/RestSharp/RestClient.Async.cs [26-42] public async Task<RestResponse> ExecuteAsync(RestRequest request, CancellationToken cancellationToken = default) { using var internalResponse = await ExecuteRequestAsync(request, cancellationToken).ConfigureAwait(false); var response = internalResponse.Exception == null ? await RestResponse.FromHttpResponse( internalResponse.ResponseMessage!, request, Options, internalResponse.CookieContainer?.GetCookies(internalResponse.Url), cancellationToken ... (clipped 7 lines) src/RestSharp/RestClient.Async.cs [46-60] public async Task<Stream?> DownloadStreamAsync(RestRequest request, CancellationToken cancellationToken = default) { // Make sure we only read the headers, so we can stream the content body efficiently request.CompletionOption = HttpCompletionOption.ResponseHeadersRead; var response = await ExecuteRequestAsync(request, cancellationToken).ConfigureAwait(false); var exception = response.Exception ?? response.ResponseMessage?.MaybeException(Options.ErrorWhenUnsuccessfulStatusCode); if (exception != null) { return Options.ThrowOnAnyError ? throw exception : null; } ... (clipped 5 lines) Solution Walkthrough: Before: // RestClient.Async.cs public async Task<RestResponse> ExecuteAsync(RestRequest request, ...) { // ... var response = await RestResponse.FromHttpResponse( ..., Options, // new ... ); return response; } public async Task<Stream?> DownloadStreamAsync(RestRequest request, ...) { // ... var exception = response.Exception ?? response.ResponseMessage?.MaybeException(Options.ErrorWhenUnsuccessfulStatusCode); // new if (exception != null) { return Options.ThrowOnAnyError ? throw exception : null; } // ... returns stream } After: // RestClient.Async.cs public async Task<RestResponse> ExecuteAsync(RestRequest request, ...) { // ... (no change) var response = await RestResponse.FromHttpResponse( ..., Options, ... ); return response; } // Suggestion: Return RestResponse instead of Stream? to provide status information public async Task<RestResponse> DownloadStreamAsync(RestRequest request, ...) { // ... var response = await ExecuteRequestAsync(request, ...); var restResponse = await RestResponse.FromHttpResponse(...); // Create a full response object // The stream can be accessed via restResponse.RawBytes or a new Stream property // The caller can now check restResponse.IsSuccessStatusCode return restResponse; } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a significant design issue where the new ErrorWhenUnsuccessfulStatusCode option introduces inconsistent error handling between ExecuteAsync and DownloadStreamAsync, which could confuse users.	Medium
General	✅ Rename property for improved clarity Suggestion Impact: The suggestion was directly implemented in the commit. Both the property name was changed from ErrorWhenUnsuccessfulStatusCode to SetErrorExceptionOnUnsuccessfulStatusCode, and the comment was updated to clarify that it controls setting the ErrorException property instead of throwing an exception. code diff: /// <summary> - /// When set to false, the client doesn't throw an exception when the response status code is not successful. + /// When set to false, the client doesn't set the `ErrorException` property for responses with unsuccessful status codes. /// Default is true. /// </summary> - public bool ErrorWhenUnsuccessfulStatusCode { get; set; } = true; + public bool SetErrorExceptionOnUnsuccessfulStatusCode { get; set; } = true; Rename the ErrorWhenUnsuccessfulStatusCode property to SetErrorExceptionOnUnsuccessfulStatusCode and update its comment to clarify that it controls setting the ErrorException property rather than directly throwing an exception. src/RestSharp/Options/RestClientOptions.cs [211-215] /// <summary> -/// When set to false, the client doesn't throw an exception when the response status code is not successful. +/// When set to false, the client doesn't set the `ErrorException` property for responses with unsuccessful status codes. /// Default is true. /// </summary> -public bool ErrorWhenUnsuccessfulStatusCode { get; set; } = true; +public bool SetErrorExceptionOnUnsuccessfulStatusCode { get; set; } = true; [Suggestion processed] Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies that the new property's name is misleading and proposes a more accurate name that better describes its function, which improves the clarity of the public API.	Low
Update

[github-actions]

Test Results

   35 files     35 suites   15m 51s ⏱️
  454 tests   454 ✅ 0 💤 0 ❌
3 068 runs  3 068 ✅ 0 💤 0 ❌

Results for commit 8a8f153.

♻️ This comment has been updated with latest results.