Skip to content

Fix ReadIOAdapter max_len handling #294

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.

Sign up for GitHub

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

Open
fallintoplace wants to merge 2 commits into
base: main
Choose a base branch
from
+69 −15
Open

Fix ReadIOAdapter max_len handling #294

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7add883
Fix ReadIOAdapter max_len handling
fallintoplace Jun 26, 2026
e352096
Handle EOF with output buffers in ReadIOAdapter
fallintoplace Jun 26, 2026
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
32 changes: 17 additions & 15 deletions lib/openai/internal/util.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,11 +428,11 @@ def close
@stream.to_a.join
in Integer
@buf << @stream.next while @buf.length < max_len
@buf.slice!(..max_len)
@buf.slice!(0, max_len)
end
rescue StopIteration
@stream = nil
@buf.slice!(0..)
@buf.empty? ? nil : @buf.slice!(0..)

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Handle EOF when read receives an output buffer

When an enumerator-backed body ends exactly on the IO.copy_stream buffer boundary, IO.copy_stream makes one more call as read(16384, outbuf). This new EOF branch now returns nil while still inside the Enumerator path, so read reaches out_string.replace(read) with read == nil and raises TypeError instead of letting Net::HTTP finish the upload. For example, an enumerator yielding exactly 16 KiB reproduces the failure through IO.copy_stream.

Useful? React with 👍 / 👎.

end

# @api private
Expand All @@ -442,21 +442,23 @@ def close
#
# @return [String, nil]
def read(max_len = nil, out_string = nil)
case @stream
in nil
nil
in IO | StringIO
@stream.read(max_len, out_string)
in Enumerator
read = read_enum(max_len)
case out_string
in String
out_string.replace(read)
read =
case @stream
in nil
read
nil
in IO | StringIO
return @stream.read(max_len, out_string).tap(&@blk)
in Enumerator
read_enum(max_len)
end
end
.tap(&@blk)

case out_string
in String
out_string.replace(read || "")
read.nil? ? nil : out_string
in nil
read
end.tap(&@blk)
end

# @api private
Expand Down
52 changes: 52 additions & 0 deletions test/openai/internal/util_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,58 @@ def test_copy_read
end
end

def test_read_respects_max_len_for_enumerator
input =
Enumerator.new do |y|
y << "ab"
y << "cd"
y << "ef"
end

# rubocop:disable Lint/EmptyBlock
adapter = OpenAI::Internal::Util::ReadIOAdapter.new(input) {}
# rubocop:enable Lint/EmptyBlock

assert_equal("abc", adapter.read(3))
assert_equal("def", adapter.read(3))
assert_nil(adapter.read(3))
end

def test_read_clears_out_string_at_eof_for_enumerator
input =
Enumerator.new do |y|
y << "ab"
y << "cd"
y << "ef"
end

# rubocop:disable Lint/EmptyBlock
adapter = OpenAI::Internal::Util::ReadIOAdapter.new(input) {}
# rubocop:enable Lint/EmptyBlock
out = +"seed"

assert_same(out, adapter.read(3, out))
assert_equal("abc", out)
assert_same(out, adapter.read(3, out))
assert_equal("def", out)
assert_nil(adapter.read(3, out))
assert_equal("", out)
end

def test_copy_read_for_enumerator_exactly_on_copy_stream_boundary
body = "a" * 16_384
input = Enumerator.new { _1 << body }
io = StringIO.new

# rubocop:disable Lint/EmptyBlock
adapter = OpenAI::Internal::Util::ReadIOAdapter.new(input) {}
# rubocop:enable Lint/EmptyBlock

IO.copy_stream(adapter, io)

assert_equal(body, io.string)
end

def test_copy_write
cases = {
StringIO.new => "",
Expand Down