Skip to content

feat(push-publishing): add per-endpoint failure details to failure events (#34356)#35765

Open
dsolistorres wants to merge 2 commits into
mainfrom
issue-34356-push-publish-failure-details
Open

feat(push-publishing): add per-endpoint failure details to failure events (#34356)#35765
dsolistorres wants to merge 2 commits into
mainfrom
issue-34356-push-publish-failure-details

Conversation

@dsolistorres
Copy link
Copy Markdown
Member

Summary

  • Enhances AllPushPublishEndpointsFailureEvent and SinglePushPublishEndpointFailureEvent with an optional List<EndpointFailureDetail> payload (new EndpointFailureDetail value object + FailureCategory enum). Subscribers can now distinguish authentication, authorization, server, network and bundle failures, capture HTTP status codes, and inspect a per-endpoint retryable hint.
  • Backward compatible: the original 1-arg constructors are preserved and delegate to the new ones with an empty list. getEndpointDetails() returns an empty (never-null) list when the legacy constructor is used, so existing subscribers compiled against the old API keep working without recompiling.
  • PushPublisher.process() records an EndpointFailureDetail at every failure branch (HTTP 401, 403, other non-2xx, missing auth key, and exception) and passes the list into the failure events at the existing emit sites — no new I/O, no schema changes.

Closes #34356

What's new

Type Location
EndpointFailureDetail (immutable, builder-based value object) dotCMS/src/main/java/com/dotcms/system/event/local/type/pushpublish/EndpointFailureDetail.java
FailureCategory enum (AUTHENTICATION, AUTHORIZATION, CLIENT_ERROR, SERVER_ERROR, NETWORK_ERROR, BUNDLE_ERROR, UNKNOWN) with from(httpStatus, auditStatus, throwable) classifier and per-category retryable default dotCMS/src/main/java/com/dotcms/system/event/local/type/pushpublish/FailureCategory.java
New 2-arg constructor on each failure event (legacy 1-arg constructor preserved, delegates with empty list) AllPushPublishEndpointsFailureEvent.java, SinglePushPublishEndpointFailureEvent.java
buildFailureDetail(...) helper + per-failure-branch capture dotCMS/src/main/java/com/dotcms/publisher/pusher/PushPublisher.java

Failure-category mapping (built into FailureCategory.from())

Trigger Category Retryable
Audit status INVALID_TOKEN / HTTP 401 / missing auth key AUTHENTICATION false
Audit status LICENSE_REQUIRED / HTTP 403 AUTHORIZATION false
HTTP 4xx other than 401/403 CLIENT_ERROR false
HTTP 5xx SERVER_ERROR true
Exception with no HTTP response NETWORK_ERROR true
Audit status FAILED_TO_BUNDLE BUNDLE_ERROR false
Fallback UNKNOWN false

Backward compatibility

Before After
Existing 1-arg constructor new AllPushPublishEndpointsFailureEvent(assets) works unchanged — delegates to new ctor with Collections.emptyList()
Existing getters (getPublishQueueElements, getName, getDate) available unchanged
getEndpointDetails() after legacy ctor n/a returns empty list (never null)
Returned list mutability n/a unmodifiable; defensive copy via Collections.unmodifiableList

External plugins compiled against the prior API continue to work without recompiling.

Subscriber usage example (what a customer plugin would do)

@Subscriber
public void onAllFailed(final AllPushPublishEndpointsFailureEvent event) {
    for (final EndpointFailureDetail d : event.getEndpointDetails()) {
        if (d.isRetryable()) {
            scheduleRetry(d);
        } else if (d.getFailureCategory() == FailureCategory.AUTHENTICATION) {
            pageOnCall("PP token expired on " + d.getEndpointName());
        } else if (d.getFailureCategory() == FailureCategory.AUTHORIZATION) {
            notifyAdmin("License required on " + d.getEndpointName());
        }
    }
}

A reference subscriber exercising the live LocalSystemEventsAPI ships in PushPublishFailureEventSubscriberTest.

Test plan

  • Unit tests added (26 new tests, all passing locally):
    • FailureCategoryTest — 13 tests covering every from(...) mapping branch and the retryable defaults
    • EndpointFailureDetailTest — 4 tests covering the builder defaults, the retryable override path, and the UNKNOWN-fallback path
    • PushPublishFailureEventsTest — 6 tests covering backward-compat constructors, payload propagation, and list-immutability on both event classes
    • PushPublishFailureEventSubscriberTest — 3 end-to-end tests wiring a real @Subscriber POJO into LocalSystemEventsAPI, firing each event variant, and asserting per-endpoint payload, category grouping, HTTP status, audit status and retryable propagate through the bus
  • ./mvnw compile -pl :dotcms-core — clean
  • ./mvnw test -pl :dotcms-core -Dtest='FailureCategoryTest,EndpointFailureDetailTest,PushPublishFailureEventsTest,PushPublishFailureEventSubscriberTest'Tests run: 26, Failures: 0, Errors: 0, Skipped: 0
  • Manual smoke: configure a remote endpoint with an invalid token, push a bundle, register a subscriber, confirm the new payload arrives with failureCategory = AUTHENTICATION and httpStatusCode = 401
  • CI build green

Notes / follow-ups

🤖 Generated with Claude Code

…ents (#34356)

Enhance AllPushPublishEndpointsFailureEvent and SinglePushPublishEndpointFailureEvent
with an optional List<EndpointFailureDetail> payload so subscribers can distinguish
authentication, authorization, server, network and bundle failures, capture HTTP
status codes, and inspect a retryable hint. Existing 1-arg constructors keep working
unchanged — getEndpointDetails() returns an empty list when the legacy constructor
is used, preserving backward compatibility for current subscribers.

PushPublisher.process() now records an EndpointFailureDetail at every failure branch
(HTTP 401, 403, other non-2xx, missing auth key, and exception) and passes the list
into the failure events at the existing emit sites.

Includes unit tests for FailureCategory.from() mapping, EndpointFailureDetail builder
defaults, backward-compat constructors on both events, and an end-to-end subscriber
test exercising the LocalSystemEventsAPI delivery path.

Refs: #34356

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 20, 2026

Claude finished @dsolistorres's task in 3m 52s —— View job


✅ dotCMS Backend Review — pipeline complete

  • Load dotCMS conventions (CLAUDE.md, JAVA_STANDARDS.md, DATABASE_PATTERNS.md, REST_API_PATTERNS.md, SECURITY_PRINCIPLES.md)
  • Get Java diff (433ea3e1...5dbe8b0a)
  • Spawn 4 specialized sub-agents (Security, Database, Java Standards, REST API)
  • Consolidate findings — all four returned NO_FINDINGS
  • Post consolidated review comment (updated marker comment #4493268458)
  • Submit formal GitHub review — blocked: I cannot submit --approve/--request-changes reviews due to GitHub App permission limits (FAQ)

Summary

All four sub-reviews came back clean:

Domain Result
🔒 Security NO_FINDINGS — no SQL, no user input, no secrets logged. EndpointFailureDetail only carries audit-safe data; exception messages are intentionally not propagated (only getClass().getName()). Events are dispatched in-JVM via LocalSystemEventsAPI, not exposed via REST.
🗄️ Database NO_FINDINGS — no DB writes, no cache mutations, no Hibernate/DotConnect usage in the diff. Pure in-memory event-payload enrichment.
☕ Java Standards NO_FINDINGS — Java 11 syntax only, all generics in place, Logger used correctly, no System.out/System.getProperty, List.copyOf() and List.of() are Java 11 compatible.
🌐 REST API NO_FINDINGS — no @Path resources touched; PushPublisher uses JAX-RS only as an outbound client.

The earlier defensive-copy concern raised on the previous push is resolved — both event classes now use List.copyOf(...), which produces a true defensive copy and an unmodifiable list.
· branch issue-34356-push-publish-failure-details

@dsolistorres dsolistorres self-assigned this May 20, 2026
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 20, 2026

✅ dotCMS Backend Review

No issues found.

Security, database, Java-standards and REST-API sub-reviews all came back clean. The previous defensive-copy concern is resolved — both event classes now use List.copyOf(...), which is a true defensive copy (and produces an unmodifiable list).

…nts (#34356)

Collections.unmodifiableList only wraps the caller's list with a read-only view —
mutations to the original list remain visible through the view. Switch to
List.copyOf so each failure event holds a true immutable snapshot of its
endpointDetails payload at construction time.

Also adds a regression test that mutates the source list after construction and
asserts the event's view stays unchanged.

Refs: #34356

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@dsolistorres dsolistorres marked this pull request as ready for review May 20, 2026 00:55
@dsolistorres dsolistorres enabled auto-merge May 20, 2026 00:56
@dsolistorres dsolistorres added this pull request to the merge queue May 20, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks May 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Backend PR changes Java/Maven backend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Enhancement: Add Failure Details to Push Publishing Events

2 participants