fix: clean up buffer on error

[dkaser]

CodeAnt-AI Description

Clear output buffer when included page errors

What Changed

• If a page included for rendering throws an error, the temporary output buffer is now discarded so no partial output remains, and the error is passed back to the caller.
• Successful includes keep returning the rendered content as before; when the file is missing the function still returns an empty string.
• This prevents leftover buffering from affecting later responses or causing unexpected output after an include fails.

Impact

✅ No leftover output buffer after page error
✅ Clearer error handling when rendering pages
✅ Fewer broken diagnostics pages due to failed includes

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

Summary by CodeRabbit

• Chores
  • Enhanced error handling for improved system robustness and stability.

[codeant-ai]

CodeAnt AI is reviewing your PR.

[coderabbitai]

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

The page.php file was modified to add try/catch error handling around a dynamic file inclusion. On success, the output buffer is returned as before. On exception, the buffer is cleaned and the exception is rethrown.

Changes

Cohort / File(s)	Summary
Error handling for dynamic includes src/usr/local/emhttp/plugins/plugin-diagnostics/include/page.php	Wrapped dynamic file inclusion in try/catch block with defensive buffer cleanup on exception

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The pull request title "fix: clean up buffer on error" is directly and accurately related to the main change in the changeset. The modifications add defensive error handling by wrapping includes in a try/catch block, with explicit buffer cleanup via ob_end_clean() when an exception occurs—exactly what the title describes. The title is specific, concise, and uses the conventional "fix:" prefix appropriately for a bug fix. It provides meaningful information that would help a teammate understand the primary purpose of the change when scanning history.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch page-buffer

---

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.}

[codeant-ai]

Suggestion: Guard calls to output-buffer functions with an ob_get_level() check so ob_get_clean() / ob_end_clean() are only called when a buffer actually exists, avoiding warnings or false failures if the buffer was closed inside the included file. [best practice]

Severity Level: Minor ⚠️

Suggested change

	return ob_get_clean() ?: "";
	} catch (\Throwable $e) {
	ob_end_clean();
	if (ob_get_level() > 0) {
	return ob_get_clean() ?: "";
	}
	return "";
	} catch (\Throwable $e) {
	if (ob_get_level() > 0) {
	ob_end_clean();
	}

Why it matters? ⭐

The suggestion is valid: the PR starts an output buffer then includes arbitrary page code that may itself manipulate or close buffers. Calling ob_get_clean()/ob_end_clean() unguarded can emit warnings or silently fail if the included file already cleaned the buffer. Wrapping those calls with ob_get_level() > 0 prevents warnings and makes the behavior deterministic. This fixes a real robustness issue rather than being merely cosmetic.

Prompt for AI Agent 🤖

<code>This is a comment left during a code review.

**Path:** src/usr/local/emhttp/plugins/plugin-diagnostics/include/page.php
**Line:** 58:60
**Comment:**
	*Best Practice: Guard calls to output-buffer functions with an `ob_get_level()` check so `ob_get_clean()` / `ob_end_clean()` are only called when a buffer actually exists, avoiding warnings or false failures if the buffer was closed inside the included file.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, please make it concise.

[codeant-ai]

Pull Request Feedback 🔍

🔒 No security issues identified

⚡ Recommended areas for review Non-catchable fatal errors The try/catch covers PHP Throwables, but some fatal errors (e.g., certain engine-level fatal errors) may not be catchable and could leave the buffer uncleared. Consider whether additional measures are needed to avoid leaving partial output in those cases. Buffer cleanup guard The new catch calls ob_end_clean() unconditionally. If there is no active output buffer (or if buffer nesting differs when included recursively), ob_end_clean() can emit warnings or disrupt other buffering. Verify correct behavior when nested buffers exist and guard calls with ob_get_level() or use a finally block that restores buffer state. Include return value ignored If an included page returns a string instead of echoing it, that return value is ignored because the code relies only on output buffering. Confirm that included pages are intended to echo output (not return content), or capture the return value of include and prefer it when present.

[codeant-ai]

CodeAnt AI finished reviewing your PR.