Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion docs/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,14 +98,20 @@ 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. |

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
Expand Down
1 change: 1 addition & 0 deletions lib/wreq_ruby/body.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 6 additions & 1 deletion lib/wreq_ruby/error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
8 changes: 8 additions & 0 deletions lib/wreq_ruby/response.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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")
Expand All @@ -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
Expand All @@ -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
Expand Down
13 changes: 7 additions & 6 deletions src/client/body/stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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)?;
Expand All @@ -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<bool, Error> {
rt::ensure_current(ruby)?;
rb_self.read_inner(ruby).map(|r| r.is_closed())
Expand All @@ -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<ReceiverStream<Bytes>, 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))
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/client/resp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -126,7 +126,7 @@ impl Response {
};
}

Err(memory_error(ruby))
Err(response_body_unavailable_error(ruby))
}
}

Expand Down
46 changes: 23 additions & 23 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<ExceptionClass> = Lazy::new(|ruby| {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -371,19 +371,19 @@ pub fn body_sender_send_error<T>(ruby: &Ruby, err: SendError<T>) -> 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",
)
}

Expand Down
3 changes: 2 additions & 1 deletion test/option_validation_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions test/stream_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down