Skip to content

Fixed IndexNow status-code detection (429/422/403 mislabelled as ping_failed)#28407

Merged
ErisDS merged 1 commit into
mainfrom
indexnow-429-detection
Jun 8, 2026
Merged

Fixed IndexNow status-code detection (429/422/403 mislabelled as ping_failed)#28407
ErisDS merged 1 commit into
mainfrom
indexnow-429-detection

Conversation

@ErisDS

@ErisDS ErisDS commented Jun 8, 2026

Copy link
Copy Markdown
Member

Summary

IndexNow failures were mis-detecting the HTTP status code, so rate-limits (429) and key-validation errors (422/403) were silently mislabelled as the generic indexnow.ping_failed (with status_code: null).

The classification in the ping() catch block keyed off err.statusCode, but for a non-2xx response got throws an HTTPError whose status lives at err.response.statusCode, not err.statusCode. So in production those branches never matched and every 429/422/403 fell through to the generic failure branch — making rate-limiting and key problems invisible in our structured logs.

Changes

  • Derive the status once and use it everywhere:
    const statusCode = err.statusCode ?? err.response?.statusCode ?? null;
    This handles both got's HTTPError (status on err.response.statusCode) and the existing manual unexpected-2xx throw (status on err.statusCode).
  • Classify on statusCode: 429 → indexnow.rate_limited; 422/403 → indexnow.key_validation_failed (403 = key not valid / key file not found — the same class of problem as 422); everything else → indexnow.ping_failed. Log the real http.response.status_code.
  • Tests: added a block that throws the real got HTTPError shape (status only on err.response.statusCode) covering 429 / 422 / 403 / 503, plus the manual-throw shape. These would have caught the original bug (the previous nock-based tests passed only because the request library happens to flatten statusCode onto the error in some versions).

No behavioural change to pinging itself — this is observability only (correct event names + status codes).

Test

  • pnpm test:single test/unit/server/services/indexnow.test.js — 27 passing.

@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4220866f-3e5c-4024-a62d-0c56d822c969

📥 Commits

Reviewing files that changed from the base of the PR and between 5624453 and feb533f.

📒 Files selected for processing (2)
  • ghost/core/core/server/services/indexnow.js
  • ghost/core/test/unit/server/services/indexnow.test.js
🚧 Files skipped from review as they are similar to previous changes (2)
  • ghost/core/core/server/services/indexnow.js
  • ghost/core/test/unit/server/services/indexnow.test.js

Walkthrough

The IndexNow ping service's error-handling logic was refactored to robustly extract the HTTP status code from multiple error shapes (err.statusCode or err.response.statusCode), then classify failures more broadly: 429 responses continue to trigger rate-limiting handling, while both 422 and 403 responses now map to key/validation-failure events. The warning log payload was updated to record the derived status code. Comprehensive regression tests validate error classification across multiple HTTP status codes and verify backward compatibility with Ghost's TooManyRequestsError shape.

Possibly related PRs

  • TryGhost/Ghost#28295: Modifies IndexNow ping error classification and test assertions around HTTP status code mapping and event logging.
  • TryGhost/Ghost#28328: Updates the same IndexNow failure logging payload fields and status code classification logic as part of a structured-logging migration.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and specifically describes the main bug fix: correcting HTTP status code detection in IndexNow that was causing 429, 422, and 403 errors to be mislabelled as generic ping failures.
Description check ✅ Passed The description is directly related to the changeset, providing clear context about the bug (status code location differences between got's HTTPError and manual throws), the fix strategy, and test coverage for the issue.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch indexnow-429-detection

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

IndexNow failures keyed off err.statusCode, but got's HTTPError exposes the
status at err.response.statusCode, so 429/422/403 fell through to the generic
indexnow.ping_failed branch (logged with status_code: null) - hiding rate
limiting and key problems.

Derive the status from both shapes (err.statusCode ?? err.response?.statusCode)
and classify: 429 rate_limited; 422/403 key_validation_failed; else ping_failed.
Tests exercise the real got HTTPError shape so they cover the production path.
@ErisDS ErisDS force-pushed the indexnow-429-detection branch from 5624453 to feb533f Compare June 8, 2026 11:24
@ErisDS ErisDS merged commit 00149cd into main Jun 8, 2026
50 checks passed
@ErisDS ErisDS deleted the indexnow-429-detection branch June 8, 2026 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant