diff --git a/docs/errors.md b/docs/errors.md index 6956421..ae220bd 100644 --- a/docs/errors.md +++ b/docs/errors.md @@ -98,7 +98,7 @@ RuntimeError | `Wreq::DecodingError` | The top-level kind is Decode. It covers parsing values, decoding response data, and response-body transport or protocol failures that wreq wraps as Decode. | | `Wreq::RedirectError` | The top-level kind is Redirect, usually because redirect policy rejected the next hop or the limit was exceeded. | | `Wreq::StatusError` | `Response#raise_for_status!` created a Status error for a 4xx or 5xx response. It has a status and no lower-level native cause. | -| `Wreq::MemoryError` | Single-use native state was consumed already or is currently borrowed. | +| `Wreq::MemoryError` | A response body operation cannot proceed, or a `BodySender` was already used for a request. This is the compatibility error for the current one-shot APIs. | | `Wreq::ForkError` | A forked child attempted to use native state inherited from its parent. See [Fork safety](fork-safety.md). | | `Wreq::InterruptError` | Ruby interrupted a native request wait. This inherits from `Interrupt`, outside the hierarchy above. | @@ -106,6 +106,12 @@ Errors created by the binding rather than by wreq have no active native predicates. The exception class still identifies the binding operation that failed. +`Wreq::MemoryError` does not report system memory exhaustion. It preserves the +current error class for one-shot APIs: a response cannot be read after it was +streamed or closed, only one response body operation can run at a time, and a +`BodySender` cannot be attached to a second request. Callers should rescue the +class instead of matching its message. + ## Predicates The top-level kind predicates are mutually exclusive. Cause-chain predicates diff --git a/lib/wreq_ruby/body.rb b/lib/wreq_ruby/body.rb index 1860d93..97234c3 100644 --- a/lib/wreq_ruby/body.rb +++ b/lib/wreq_ruby/body.rb @@ -17,6 +17,7 @@ module Wreq # # A sender can be attached to one request. Closing it prevents further writes but # retains queued chunks so a request attached afterward can still drain them. + # Attaching the same sender to another request raises Wreq::MemoryError. # Creating or using a sender raises Wreq::ForkError if the child inherited # wreq-ruby from its parent. class BodySender diff --git a/lib/wreq_ruby/error.rb b/lib/wreq_ruby/error.rb index be90077..e9e8a35 100644 --- a/lib/wreq_ruby/error.rb +++ b/lib/wreq_ruby/error.rb @@ -149,7 +149,12 @@ def upgrade? # never swallows a Ruby interrupt. class InterruptError < Interrupt; end - # Raised when single-use native state was already consumed or is borrowed. + # Raised when a response body operation cannot proceed or a one-shot + # request-body sender cannot be used again. + # + # This compatibility error covers the current response and BodySender APIs. + # Its message describes the Ruby object state without exposing native + # ownership or borrowing details. # # @example A closed response no longer has a readable body # response = Wreq.get("https://example.com") diff --git a/lib/wreq_ruby/response.rb b/lib/wreq_ruby/response.rb index c107359..c41ed93 100644 --- a/lib/wreq_ruby/response.rb +++ b/lib/wreq_ruby/response.rb @@ -122,6 +122,8 @@ def cookies # Get the response bytes as a binary string. # @return [String] Response body as binary data + # @raise [Wreq::MemoryError] if another body operation is active, or the + # body was streamed or closed # @raise [Wreq::ForkError] if the child inherited wreq-ruby from its parent # @example # binary_data = response.bytes @@ -137,6 +139,8 @@ def bytes # @example # html = response.text("ISO-8859-1") # puts html + # @raise [Wreq::MemoryError] if another body operation is active, or the + # body was streamed or closed # @raise [Wreq::DecodingError] if body cannot be decoded with the specified encoding # @raise [Wreq::ForkError] if the child inherited wreq-ruby from its parent def text(default_encoding = "UTF-8") @@ -148,6 +152,8 @@ def text(default_encoding = "UTF-8") # values. Fractional and exponent-form numbers are returned as Float values. # # @return [Object] Parsed JSON (Hash, Array, String, Integer, Float, Boolean, nil) + # @raise [Wreq::MemoryError] if another body operation is active, or the + # body was streamed or closed # @raise [Wreq::DecodingError] if body is not valid JSON # @raise [Wreq::ForkError] if the child inherited wreq-ruby from its parent # @example @@ -165,6 +171,8 @@ def json # @return [nil] # @yield [chunk] Each chunk of the response body as a binary String # @raise [LocalJumpError] if called without a block + # @raise [Wreq::MemoryError] if another body operation is active, or the + # body was already read, streamed, or closed # @raise [Wreq::TimeoutError, Wreq::BodyError, Wreq::ConnectionResetError, Wreq::RequestError] # if streaming fails while reading the response body # @raise [Wreq::ForkError] if the child inherited wreq-ruby from its parent diff --git a/src/client/body/stream.rs b/src/client/body/stream.rs index d3d2807..d55aaea 100644 --- a/src/client/body/stream.rs +++ b/src/client/body/stream.rs @@ -16,7 +16,8 @@ use crate::{ arch::ProcessLocal, error::{ argument_error, body_sender_borrow_error, body_sender_borrow_mut_error, - body_sender_send_error, closed_body_sender_error, memory_error, type_error, wreq_error, + body_sender_reused_error, body_sender_send_error, closed_body_sender_error, type_error, + wreq_error, }, rt, }; @@ -140,7 +141,7 @@ impl BodySender { /// # Errors /// /// Returns `Wreq::ForkError` before reading an inherited channel, or - /// `Wreq::BodyError` if the internal state is already borrowed. + /// `Wreq::BodyError` if the internal state is already in use. pub fn close(ruby: &Ruby, rb_self: &Self) -> Result<(), Error> { rt::ensure_current(ruby)?; let mut inner = rb_self.write_inner(ruby)?; @@ -153,7 +154,7 @@ impl BodySender { /// # Errors /// /// Returns `Wreq::ForkError` before reading an inherited channel, or - /// `Wreq::BodyError` if the internal state is already borrowed. + /// `Wreq::BodyError` if the internal state is already in use. pub fn is_closed(ruby: &Ruby, rb_self: &Self) -> Result { rt::ensure_current(ruby)?; rb_self.read_inner(ruby).map(|r| r.is_closed()) @@ -179,15 +180,15 @@ impl BodySender { /// /// # Errors /// - /// Returns `Wreq::MemoryError` if the receiver was already consumed, or - /// `Wreq::BodyError` if Ruby re-enters while the state is borrowed. + /// Returns `Wreq::MemoryError` if the sender was already used for a request, or + /// `Wreq::BodyError` if Ruby re-enters while the state is in use. pub(super) fn take_receiver(&self, ruby: &Ruby) -> Result, Error> { rt::ensure_current(ruby)?; self.write_inner(ruby)? .rx .take() .map(ReceiverStream::new) - .ok_or_else(|| memory_error(ruby)) + .ok_or_else(|| body_sender_reused_error(ruby)) } } diff --git a/src/client/resp.rs b/src/client/resp.rs index db2223a..4c3f52b 100644 --- a/src/client/resp.rs +++ b/src/client/resp.rs @@ -12,7 +12,7 @@ use crate::{ arch::ProcessLocal, client::body::{json::Json, stream::BodyReceiver}, cookie::Cookie, - error::{memory_error, no_block_given_error, wreq_error}, + error::{no_block_given_error, response_body_unavailable_error, wreq_error}, gvl, header::Headers, http::{StatusCode, Version}, @@ -126,7 +126,7 @@ impl Response { }; } - Err(memory_error(ruby)) + Err(response_body_unavailable_error(ruby)) } } diff --git a/src/error.rs b/src/error.rs index 00a0c01..3c3e2a5 100644 --- a/src/error.rs +++ b/src/error.rs @@ -14,20 +14,6 @@ use tokio::sync::mpsc::error::SendError; const ERROR_PREDICATES_IVAR: &str = "wreq_error_predicates"; type ErrorPredicateBits = u64; -const RACE_CONDITION_ERROR_MSG: &str = r#"Due to Rust's memory management with borrowing, -you cannot use certain instances multiple times as they may be consumed. - -This error can occur in the following cases: -1) You passed a non-clonable instance to a function that requires ownership. -2) You attempted to use a method that consumes ownership more than once (e.g., reading a response body twice). -3) You tried to reference an instance after it was borrowed. - -Potential solutions: -1) Avoid sharing instances; create a new instance each time you use it. -2) Refrain from performing actions that consume ownership multiple times. -3) Change the order of operations to reference the instance before borrowing it. -"#; - macro_rules! define_exception { ($name:ident, $ruby_name:literal, $parent_method:ident) => { static $name: Lazy = Lazy::new(|ruby| { @@ -315,9 +301,23 @@ define_exception!(INTERRUPT_ERROR, "InterruptError", exception_interrupt); define_exception!(MEMORY, "MemoryError", exception_runtime_error); define_exception!(FORK_ERROR, "ForkError", exception_runtime_error); -/// Memory error constant -pub fn memory_error(ruby: &Ruby) -> MagnusError { - MagnusError::new(ruby.get_inner(&MEMORY), RACE_CONDITION_ERROR_MSG) +// Keep these constructors separate even though both currently use MemoryError. +// A future Ruby body API can change either state error without inspecting or +// coupling itself to the native storage mechanism. +/// Build the compatibility error used when a response body operation cannot proceed. +pub fn response_body_unavailable_error(ruby: &Ruby) -> MagnusError { + MagnusError::new( + ruby.get_inner(&MEMORY), + "response body is unavailable for this operation", + ) +} + +/// Build the compatibility error used when a sender is reused for another request. +pub fn body_sender_reused_error(ruby: &Ruby) -> MagnusError { + MagnusError::new( + ruby.get_inner(&MEMORY), + "body sender has already been used for a request", + ) } /// Create a `Wreq::InterruptError` when Ruby interrupts a request. @@ -371,19 +371,19 @@ pub fn body_sender_send_error(ruby: &Ruby, err: SendError) -> MagnusError ) } -/// Map an immutable sender-state borrow failure to `Wreq::BodyError`. -pub fn body_sender_borrow_error(ruby: &Ruby, err: BorrowError) -> MagnusError { +/// Map an immutable sender-state access conflict to `Wreq::BodyError`. +pub fn body_sender_borrow_error(ruby: &Ruby, _err: BorrowError) -> MagnusError { MagnusError::new( ruby.get_inner(&BODY_ERROR), - format!("body sender state is unavailable: {err}"), + "body sender is currently in use", ) } -/// Map a mutable sender-state borrow failure to `Wreq::BodyError`. -pub fn body_sender_borrow_mut_error(ruby: &Ruby, err: BorrowMutError) -> MagnusError { +/// Map a mutable sender-state access conflict to `Wreq::BodyError`. +pub fn body_sender_borrow_mut_error(ruby: &Ruby, _err: BorrowMutError) -> MagnusError { MagnusError::new( ruby.get_inner(&BODY_ERROR), - format!("body sender state is unavailable: {err}"), + "body sender is currently in use", ) } diff --git a/test/option_validation_test.rb b/test/option_validation_test.rb index fce6e06..353fae6 100644 --- a/test/option_validation_test.rb +++ b/test/option_validation_test.rb @@ -116,7 +116,8 @@ def test_consumed_body_sender_error_preserves_class_and_names_option Wreq.post(INVALID_URL, body: sender) end - assert_includes error.message, ":body" + assert_equal "invalid value for :body: body sender has already been used for a request", + error.message ensure sender&.close end diff --git a/test/stream_test.rb b/test/stream_test.rb index c444756..1e78882 100644 --- a/test/stream_test.rb +++ b/test/stream_test.rb @@ -303,6 +303,7 @@ def test_chunks_called_twice_raises_error error_raised = true assert_instance_of Wreq::MemoryError, e, "Second chunks call should raise MemoryError, got #{e.class}: #{e.message}" + assert_equal "response body is unavailable for this operation", e.message end assert error_raised, "Second chunks call should raise an error" @@ -320,6 +321,7 @@ def test_text_after_chunks_raises_error error_raised = true assert_instance_of Wreq::MemoryError, e, "Calling text after chunks should raise MemoryError, got #{e.class}: #{e.message}" + assert_equal "response body is unavailable for this operation", e.message end assert error_raised, "Calling text after chunks should raise an error"