Skip to content

Commit 4c27441

Browse files
yhiroseyhirose
committed
Enhance BodyReader error handling with last_error tracking and add corresponding tests
1 parent 6137446 commit 4c27441

File tree

2 files changed

+177
-4
lines changed

2 files changed

+177
-4
lines changed

‎httplib.h‎

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1467,9 +1467,15 @@ struct BodyReader {
14671467
// For chunked encoding
14681468
size_t current_chunk_remaining = 0;
14691469

1470+
// Error tracking
1471+
Error last_error = Error::Success;
1472+
14701473
// Read up to len bytes into buf
14711474
// Returns bytes read, 0 on EOF, -1 on error
14721475
ssize_t read(char *buf, size_t len);
1476+
1477+
// Check if an error occurred during reading
1478+
bool has_error() const { return last_error != Error::Success; }
14731479
};
14741480

14751481
} // namespace detail
@@ -1568,6 +1574,12 @@ class ClientImpl {
15681574
return result;
15691575
}
15701576
}
1577+
1578+
// Get the last error that occurred during reading (socket direct mode only)
1579+
Error get_read_error() const { return body_reader_.last_error; }
1580+
1581+
// Check if a read error occurred (socket direct mode only)
1582+
bool has_read_error() const { return body_reader_.has_error(); }
15711583
};
15721584

15731585
// clang-format off
@@ -7255,7 +7267,10 @@ inline ssize_t Stream::write(const std::string &s) {
72557267

72567268
// BodyReader implementation
72577269
inline ssize_t detail::BodyReader::read(char *buf, size_t len) {
7258-
if (!stream) { return -1; }
7270+
if (!stream) {
7271+
last_error = Error::Connection;
7272+
return -1;
7273+
}
72597274
if (eof) { return 0; }
72607275

72617276
if (!chunked) {
@@ -7269,10 +7284,17 @@ inline ssize_t detail::BodyReader::read(char *buf, size_t len) {
72697284
auto to_read = (std::min)(len, remaining);
72707285
auto n = stream->read(buf, to_read);
72717286

7272-
if (n <= 0) {
7287+
if (n < 0) {
7288+
last_error = Error::Read;
72737289
eof = true;
72747290
return n;
72757291
}
7292+
if (n == 0) {
7293+
// Unexpected EOF before content_length
7294+
last_error = Error::Read;
7295+
eof = true;
7296+
return 0;
7297+
}
72767298

72777299
bytes_read += static_cast<size_t>(n);
72787300
if (bytes_read >= content_length) { eof = true; }
@@ -7288,6 +7310,7 @@ inline ssize_t detail::BodyReader::read(char *buf, size_t len) {
72887310
stream_line_reader line_reader(*stream, line_buf, line_buf_size);
72897311

72907312
if (!line_reader.getline()) {
7313+
last_error = Error::Read;
72917314
eof = true;
72927315
return 0;
72937316
}
@@ -7302,7 +7325,8 @@ inline ssize_t detail::BodyReader::read(char *buf, size_t len) {
73027325
char *end_ptr;
73037326
auto chunk_size = std::strtoul(line_reader.ptr(), &end_ptr, 16);
73047327
if (end_ptr == line_reader.ptr() || chunk_size == ULONG_MAX) {
7305-
return -1; // Parse error
7328+
last_error = Error::Read; // Invalid chunk format
7329+
return -1;
73067330
}
73077331

73087332
if (chunk_size == 0) {
@@ -7318,10 +7342,17 @@ inline ssize_t detail::BodyReader::read(char *buf, size_t len) {
73187342
auto to_read = (std::min)(len, current_chunk_remaining);
73197343
auto n = stream->read(buf, to_read);
73207344

7321-
if (n <= 0) {
7345+
if (n < 0) {
7346+
last_error = Error::Read;
73227347
eof = true;
73237348
return n;
73247349
}
7350+
if (n == 0) {
7351+
// Unexpected EOF in chunk
7352+
last_error = Error::Read;
7353+
eof = true;
7354+
return 0;
7355+
}
73257356

73267357
current_chunk_remaining -= static_cast<size_t>(n);
73277358
bytes_read += static_cast<size_t>(n);

‎test/test20.cc‎

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -701,6 +701,108 @@ TEST(BodyReaderTest, ReadWithoutStream) {
701701
auto n = reader.read(buf, sizeof(buf));
702702

703703
EXPECT_EQ(-1, n);
704+
EXPECT_TRUE(reader.has_error());
705+
EXPECT_EQ(httplib::Error::Connection, reader.last_error);
706+
}
707+
708+
//------------------------------------------------------------------------------
709+
// Phase 2.10: Error handling tests
710+
//------------------------------------------------------------------------------
711+
712+
// Mock stream that returns error after N bytes
713+
class ErrorAfterNBytesStream : public httplib::Stream {
714+
public:
715+
ErrorAfterNBytesStream(const std::string &data, size_t error_after)
716+
: data_(data), pos_(0), error_after_(error_after) {}
717+
718+
bool is_readable() const override { return true; }
719+
bool wait_readable() const override { return true; }
720+
bool wait_writable() const override { return true; }
721+
722+
ssize_t read(char *ptr, size_t size) override {
723+
if (pos_ >= error_after_) { return -1; } // Simulate read error
724+
if (pos_ >= data_.size()) { return 0; }
725+
size_t to_read =
726+
std::min(size, std::min(data_.size() - pos_, error_after_ - pos_));
727+
std::memcpy(ptr, data_.data() + pos_, to_read);
728+
pos_ += to_read;
729+
return static_cast<ssize_t>(to_read);
730+
}
731+
732+
ssize_t write(const char *, size_t) override { return -1; }
733+
734+
void get_remote_ip_and_port(std::string &ip, int &port) const override {
735+
ip = "127.0.0.1";
736+
port = 0;
737+
}
738+
739+
void get_local_ip_and_port(std::string &ip, int &port) const override {
740+
ip = "127.0.0.1";
741+
port = 0;
742+
}
743+
744+
socket_t socket() const override { return INVALID_SOCKET; }
745+
time_t duration() const override { return 0; }
746+
747+
private:
748+
std::string data_;
749+
size_t pos_;
750+
size_t error_after_;
751+
};
752+
753+
TEST(BodyReaderErrorTest, ReadErrorSetsLastError) {
754+
ErrorAfterNBytesStream stream("Hello, World!", 5);
755+
756+
httplib::detail::BodyReader reader;
757+
reader.stream = &stream;
758+
reader.content_length = 13;
759+
reader.chunked = false;
760+
761+
char buf[32];
762+
763+
// First read succeeds (5 bytes)
764+
auto n1 = reader.read(buf, sizeof(buf));
765+
EXPECT_EQ(5, n1);
766+
EXPECT_FALSE(reader.has_error());
767+
768+
// Second read fails
769+
auto n2 = reader.read(buf, sizeof(buf));
770+
EXPECT_EQ(-1, n2);
771+
EXPECT_TRUE(reader.has_error());
772+
EXPECT_EQ(httplib::Error::Read, reader.last_error);
773+
}
774+
775+
TEST(BodyReaderErrorTest, UnexpectedEOFSetsError) {
776+
// Data is shorter than content_length
777+
MockStream stream("Short");
778+
779+
httplib::detail::BodyReader reader;
780+
reader.stream = &stream;
781+
reader.content_length = 100; // Expect 100 bytes but only 5 available
782+
reader.chunked = false;
783+
784+
char buf[32];
785+
786+
// First read gets the available data
787+
auto n1 = reader.read(buf, sizeof(buf));
788+
EXPECT_EQ(5, n1);
789+
EXPECT_FALSE(reader.has_error());
790+
791+
// Second read hits unexpected EOF
792+
auto n2 = reader.read(buf, sizeof(buf));
793+
EXPECT_EQ(0, n2);
794+
EXPECT_TRUE(reader.has_error());
795+
EXPECT_EQ(httplib::Error::Read, reader.last_error);
796+
}
797+
798+
TEST(BodyReaderErrorTest, HasErrorMethod) {
799+
httplib::detail::BodyReader reader;
800+
801+
EXPECT_FALSE(reader.has_error());
802+
EXPECT_EQ(httplib::Error::Success, reader.last_error);
803+
804+
reader.last_error = httplib::Error::Read;
805+
EXPECT_TRUE(reader.has_error());
704806
}
705807

706808
// =============================================================================
@@ -801,6 +903,46 @@ TEST_F(StreamHandleV2Test, ReadAllSocketDirect) {
801903
EXPECT_EQ("Hello from socket!", result);
802904
}
803905

906+
TEST_F(StreamHandleV2Test, GetReadErrorReturnsSuccess) {
907+
httplib::ClientImpl::StreamHandle handle;
908+
909+
handle.response = std::make_unique<httplib::Response>();
910+
handle.response->status = 200;
911+
912+
handle.stream_ = mock_stream_.get();
913+
handle.body_reader_.stream = mock_stream_.get();
914+
handle.body_reader_.content_length = 18;
915+
916+
// No error initially
917+
EXPECT_FALSE(handle.has_read_error());
918+
EXPECT_EQ(httplib::Error::Success, handle.get_read_error());
919+
920+
// Read successfully
921+
handle.read_all();
922+
EXPECT_FALSE(handle.has_read_error());
923+
}
924+
925+
TEST_F(StreamHandleV2Test, GetReadErrorAfterReadFailure) {
926+
auto error_stream =
927+
std::make_unique<ErrorAfterNBytesStream>("Hello World", 5);
928+
929+
httplib::ClientImpl::StreamHandle handle;
930+
handle.response = std::make_unique<httplib::Response>();
931+
handle.response->status = 200;
932+
933+
handle.stream_ = error_stream.get();
934+
handle.body_reader_.stream = error_stream.get();
935+
handle.body_reader_.content_length = 11;
936+
937+
// Read until error
938+
char buf[32];
939+
handle.read(buf, sizeof(buf)); // First read: 5 bytes OK
940+
handle.read(buf, sizeof(buf)); // Second read: error
941+
942+
EXPECT_TRUE(handle.has_read_error());
943+
EXPECT_EQ(httplib::Error::Read, handle.get_read_error());
944+
}
945+
804946
// =============================================================================
805947
// Phase 2.5: open_stream_direct() Tests (True Streaming)
806948
// =============================================================================

0 commit comments

Comments
 (0)