Dietwall fix msgpack to json

[dietWall]

slightly better error handling on internal errors

first work for #11078
however, this error implies that more work is needed.

Enter [N/A] in the box, if an item is not applicable to your change.

Testing
Before we can approve your change; please submit the following in a comment:

• Example configuration file for the change
• Debug log output from testing the change

• Attached Valgrind output that shows no leaks or memory corruption was found

If this is a change to packaging of containers or native binaries then please confirm it works for all targets.

• Run local packaging test showing all targets (including any new ones) build.
• Set ok-package-test label to test for all targets (requires maintainer to do).

Documentation

• Documentation required for this feature

Backporting

• Backport to latest stable release.

---

Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.

Summary by CodeRabbit

Release Notes

• Bug Fixes
  • Improved error handling and retry logic for message packing operations to prevent infinite retries
  • Enhanced logging for buffer allocation during retry scenarios
  • Better handling of empty data cases with appropriate cleanup and warning messages

[coderabbitai]

Walkthrough

The change modifies error handling in the flb_msgpack_to_json_str function within src/flb_pack.c. The retry condition is narrowed from ret <= 0 to ret < 0, and explicit handling is added for the ret == 0 case, which now triggers a warning log, buffer cleanup, and NULL return instead of potential infinite retries.

Changes

Cohort / File(s)	Change Summary
Msgpack error handling refinement src/flb_pack.c	Modified retry logic in flb_msgpack_to_json_str: narrowed retry condition from ret <= 0 to ret < 0, added logging on retry with buffer size, and introduced explicit handling for ret == 0 to log warning, free buffer, and return NULL instead of falling through.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Focus on verifying that the new ret == 0 handler correctly frees memory and prevents infinite retry loops
• Confirm that narrowing the retry condition from ret <= 0 to ret < 0 doesn't mask legitimate error states elsewhere in the codebase

Suggested reviewers

• koleini
• fujimotos

Poem

A rabbit hops through error trails, 🐰
Where retry loops once told long tales,
Now zero cases find their way,
With cleaner paths and logs to say,
"No data packed—we'll exit clean!"

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title "Dietwall fix msgpack to json" is partially related to the changeset. It clearly refers to msgpack-to-json conversion, which is the actual area being modified, though it doesn't capture the specific nature of the changes—namely, improvements to error handling and retry logic. The term "fix" is somewhat generic, and the inclusion of "Dietwall" (the author's username) adds unnecessary noise to the title. However, the title does convey meaningful information about what part of the codebase is being changed, and it's sufficiently clear that a teammate scanning history would understand the general area of impact, even if the specific details require further investigation.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Stop retrying when msgpack buffer is too small

The loop used to treat any non‑positive return from flb_msgpack_to_json as “buffer too small” and double the buffer until the conversion succeeded. Now only < 0 retries, while a zero result drops into this branch, logs a warning and bails out. However flb_msgpack_to_json returns 0 (not a negative value) whenever the output buffer lacks space, so this change aborts instead of reallocating. Any call that passes a modest initial size will now fail for larger records where it previously succeeded. Consider restoring the retry when ret == 0 so the function still grows the buffer.

Useful? React with 👍 / 👎.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

src/flb_pack.c (2)

1481-1500: Retry logic change prevents infinite loops but may need refinement.

The change from ret <= 0 to ret < 0 with explicit ret == 0 handling prevents infinite retry loops, which is good. However, this also means legitimate "buffer too small" cases (where flb_msgpack_to_json returns 0) no longer trigger retries.

Consider these improvements:

1. Add a retry counter to prevent infinite loops while still allowing reasonable retries:

   int retry_count = 0;
   int max_retries = 10;
   
   while (retry_count < max_retries) {
       ret = flb_msgpack_to_json(buf, size, obj, escape_unicode);
       if (ret < 0 || ret == 0) {
           size *= 2;
           retry_count++;
           // ... realloc logic
       }
       else {
           break;
       }
   }
   if (retry_count >= max_retries) {
       flb_warn("max retries reached in msgpack-to-json conversion");
       flb_free(buf);
       return NULL;
   }

2. Track progress by checking if any bytes were written between iterations to detect true failure vs. insufficient buffer.

The current approach is a reasonable first step given the PR acknowledges more work is needed for issue #11078.

Based on learnings: The PR objectives indicate this is incomplete work, so the current trade-off (safety over functionality) aligns with incremental improvement.

---

1484-1484: Consider enhancing the log message with return value context.

The log message is helpful, but including the return value could aid debugging:

-            flb_info("failed to convert msgpack to json, retrying with new size=%zu", size);
+            flb_info("failed to convert msgpack to json (ret=%d), retrying with new size=%zu", ret, size);

This would help distinguish between different error scenarios in logs.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6345fd1 and e08b45a.

📒 Files selected for processing (1)

• src/flb_pack.c (2 hunks)

🧰 Additional context used

🧠 Learnings (1)

📓 Common learnings

Learnt from: shadowshot-x
PR: fluent/fluent-bit#10825
File: plugins/out_s3/s3.c:1339-1344
Timestamp: 2025-09-04T12:35:22.872Z
Learning: In the Fluent Bit S3 plugin, the user prefers to maintain current retry_limit behavior without special handling for FLB_OUT_RETRY_UNLIMITED (-1), as there's no documentation indicating -1 should be used for infinite retries and consistency with current logic is preferred.

Learnt from: shadowshot-x
PR: fluent/fluent-bit#10825
File: plugins/out_s3/s3.c:3275-3282
Timestamp: 2025-09-04T12:35:36.904Z
Learning: The out_s3 plugin intentionally uses a simple numeric comparison for retry_limit (chunk->failures >= ctx->ins->retry_limit) rather than the standard Fluent Bit pattern that checks for FLB_OUT_RETRY_UNLIMITED (-1). The maintainer wants to keep this current behavior for consistency within the plugin.

🧬 Code graph analysis (1)

src/flb_pack.c (1)

include/fluent-bit/flb_mem.h (1)

• flb_free (126-128)

[dietWall]

7 tests failed.
Looking forward to fix them.