More details in snapshot fetch errors

[achamayou]

Prompted by the 400 fetch failures spotted in #7636.

[achamayou]

Error during snapshot fetch: Expected PARTIAL_CONTENT response from GET https://127.173.195.102:42429/node/snapshot/snapshot_104_105.committed, instead received 400 (No error)

[Copilot]

Pull request overview

This PR enhances error reporting for snapshot fetch operations by including the HTTP response body in error messages when unexpected status codes are received. This change was prompted by 400 fetch failures observed in issue #7636, where additional context about the error would help with debugging.

Changes:

• Modified the EXPECT_HTTP_RESPONSE_STATUS macro to accept a response_body parameter and include its content in error messages
• Updated both call sites of the macro to pass the response body pointer

[Copilot]

If the response buffer contains binary data or non-UTF8 content, constructing a string directly from the buffer could result in invalid UTF-8 sequences or unprintable characters in the error message. Consider sanitizing the content for display, for example by using a hex dump for binary data or escaping non-printable characters.

Suggested change

	std::string response_content( \
	response_body->buffer.begin(), response_body->buffer.end()); \
	std::string response_content; \
	bool has_non_printable = false; \
	for (const auto& ch : response_body->buffer) \
	{ \
	unsigned char uch = static_cast<unsigned char>(ch); \
	if ((uch >= 0x20 && uch <= 0x7e) || uch == '\r' || uch == '\n' || uch == '\t') \
	{ \
	response_content.push_back(static_cast<char>(uch)); \
	} \
	else \
	{ \
	has_non_printable = true; \
	break; \
	} \
	} \
	if (has_non_printable) \
	{ \
	response_content = fmt::format( \
	"<{} bytes of non-text response data>", response_body->buffer.size()); \
	} \

[Copilot]

The response_body pointer could be null according to the CurlRequest constructor (line 456-463 in curl.h). Dereferencing it without a null check will cause a crash. Consider adding a null check before accessing response_body->buffer, or handle the case where response_body is null by providing a default message like "(no response body)".

Suggested change

	std::string response_content( \
	response_body->buffer.begin(), response_body->buffer.end()); \
	std::string response_content; \
	if (response_body != nullptr) \
	{ \
	response_content.assign( \
	response_body->buffer.begin(), response_body->buffer.end()); \
	} \
	else \
	{ \
	response_content = "(no response body)"; \
	} \

[Copilot]

Constructing a string from the entire response buffer could be problematic for large responses or binary data. Since snapshots can be "extremely large" (as noted in the comment at lines 198-199), this could cause memory issues or create an unwieldy error message. Consider either: (1) truncating the response content to a reasonable size (e.g., first 500 bytes), or (2) only including the response body for specific error codes that are expected to have small, informative responses (like 400 Bad Request).