Skip to content

[Bug]: Gotenberg health ping instead of try convert dummy file#18954

Merged
kingjia90 merged 13 commits into12.3from
fix-gotenberg-ping
Feb 9, 2026
Merged

[Bug]: Gotenberg health ping instead of try convert dummy file#18954
kingjia90 merged 13 commits into12.3from
fix-gotenberg-ping

Conversation

@kingjia90
Copy link
Copy Markdown
Contributor

Changes in this pull request

Resolves #18945

Additional info

A more properly and efficient health/ping check based on https://gotenberg.dev/docs/routes#health-check-route

@kingjia90 kingjia90 added this to the 12.3.2 milestone Jan 28, 2026
Copilot AI review requested due to automatic review settings January 28, 2026 14:53
@kingjia90 kingjia90 changed the title ping health instead of try convert dummy file [Bug]: Gotenberg health ping instead of try convert dummy file Jan 28, 2026
@github-actions
Copy link
Copy Markdown

Review Checklist

  • Target branch (12.3 for bug fixes, others 12.x)
  • Tests (if it's testable code, there should be a test for it - get help)
  • Docs (every functionality needs to be documented, see here)
  • Migration incl. install.sql (e.g. if the database schema changes, ...)
  • Upgrade notes (deprecations, important information, migration hints, ...)
  • Label
  • Milestone

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request addresses a critical performance issue where unconfigured Gotenberg services cause 30+ second delays when loading PDF assets. The change replaces the inefficient file conversion health check with a proper HTTP health ping to Gotenberg's /health endpoint, with aggressive timeouts (1s connect, 2s total) to prevent long delays.

Changes:

  • Replaced dummy file conversion test with direct /health endpoint ping
  • Added configurable short timeouts (1s connect, 2s overall) for health checks
  • Introduced new Guzzle HTTP client with custom timeout configuration

kingjia90 and others added 2 commits January 28, 2026 16:05
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings January 28, 2026 15:08
@kingjia90 kingjia90 marked this pull request as draft January 28, 2026 15:12
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

@kingjia90 kingjia90 marked this pull request as ready for review January 28, 2026 15:19
Copilot AI review requested due to automatic review settings January 28, 2026 15:19
@kingjia90

This comment was marked as outdated.

@kingjia90 kingjia90 marked this pull request as draft January 28, 2026 15:22
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 7 comments.

@kingjia90 kingjia90 self-assigned this Jan 29, 2026
kingjia90 and others added 2 commits January 29, 2026 09:25
@kingjia90 kingjia90 marked this pull request as ready for review January 29, 2026 09:38
Copilot AI review requested due to automatic review settings January 29, 2026 09:38
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 6 comments.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings January 29, 2026 10:25
Removed unnecessary docblock for isAvailable method.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 6 comments.

Comments suppressed due to low confidence (2)

lib/Helper/GotenbergHelper.php:87

  • Caching the unavailable status is a good improvement that addresses the performance issue mentioned in #18945. However, the cache uses the same TTL for both available and unavailable status. Consider using a shorter TTL for the unavailable status (e.g., 5-10 seconds) while keeping a longer TTL for available status (60 seconds). This would allow the system to detect when Gotenberg becomes available more quickly while still preventing the 30+ second delays when it's unavailable.
}

lib/Helper/GotenbergHelper.php:90

  • The GotenbergHelper class lacks test coverage. Given that this change addresses a critical performance issue (30+ second delays from issue #18945) and introduces new caching behavior with health check endpoints, it would be beneficial to add tests that verify: 1) the caching behavior for both available and unavailable states, 2) the health check endpoint functionality, 3) handling of missing configuration. Other helper classes in the codebase (ParameterBagHelper, CsvFormulaTest) have comprehensive test coverage.
class GotenbergHelper
{
    private static ?bool $validPing = null;

    private const CACHE_KEY = 'gotenberg_ping';
    private const STATUS_AVAILABLE = 'available';
    private const STATUS_UNAVAILABLE = 'unavailable';

    private static function healthPing(): bool
    {
        $gotenbergBaseUrl = Config::getSystemConfiguration('gotenberg')['base_url'];
        if ($gotenbergBaseUrl) {
            try {
                $ch = curl_init(rtrim($gotenbergBaseUrl, '/') . '/health');

                curl_setopt_array($ch, [
                    CURLOPT_RETURNTRANSFER => true,
                    CURLOPT_TIMEOUT => 2,
                    CURLOPT_CONNECTTIMEOUT => 2,
                ]);

                curl_exec($ch);
                $status = curl_getinfo($ch, CURLINFO_HTTP_CODE);
                curl_close($ch);

                return $status === 200;
            } catch (\Throwable $e) {
                return false;
            }
        }
        return false;
    }

    public static function isAvailable(): bool
    {
        if (self::$validPing !== null) {
            return self::$validPing;
        }

        $cachedStatus = Cache::load(self::CACHE_KEY);

        if ($cachedStatus === self::STATUS_AVAILABLE) {
            self::$validPing = true;
            return true;
        }

        if ($cachedStatus === self::STATUS_UNAVAILABLE) {
            self::$validPing = false;
            return false;
        }

        $ttl = Config::getSystemConfiguration('gotenberg')['ping_cache_ttl'];

        if (self::healthPing()) {
            self::$validPing = true;
            Cache::save(self::STATUS_AVAILABLE, self::CACHE_KEY, [], $ttl);
            return true;
        }

        self::$validPing = false;
        Cache::save(self::STATUS_UNAVAILABLE, self::CACHE_KEY, [], $ttl);
        return false;
    }
}

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings January 30, 2026 07:42
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

@robertSt7 robertSt7 modified the milestones: 12.3.2, 12.3.3 Feb 4, 2026
@kingjia90 kingjia90 merged commit a7e6f3c into 12.3 Feb 9, 2026
14 checks passed
@kingjia90 kingjia90 deleted the fix-gotenberg-ping branch February 9, 2026 08:11
@github-actions github-actions bot locked and limited conversation to collaborators Feb 9, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants