Skip to content

fix(sdk): preserve type-only callback failure messages - #851

Merged
zhongkechen merged 4 commits into
aws:mainfrom
YingzuoLiu:fix/callback-type-only-message-849
Oct 30, 2026
Merged

zhongkechen merged 4 commits into
aws:mainfrom
YingzuoLiu:fix/callback-type-only-message-849

Conversation

@YingzuoLiu

@YingzuoLiu YingzuoLiu commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Issue Link, if available

Fixes #849.

Description

A callback failed with only ErrorType="RejectedError" produces "RejectedError: null", which also reaches the telemetry exporter's fallback message. Return the supplied type alone when the message is absent or empty. An ErrorObject whose type and message are both absent or empty now displays "Callback failed". Preserve the original ErrorObject fields, exception class, callback identity, and replay behavior.

Design

  • Design note: not needed; isolated bug fix in a private formatting helper, no public API members or SDK invariants change.
  • Root cause: buildMessage concatenates a present error type with an absent/empty message, and can return null or an empty string when both fields lack content.
  • Simpler options considered: exporter-only cleanup would leave callers and checkpointed containing errors malformed; mutating ErrorObject would lose the original absent field. Fix the display message once in the exception constructor.
  • Production source: +6 lines; public API members: unchanged.

Assumptions

  • Type-only callback failure and replay behavior are verified through the real SDK and LocalDurableTestRunner, using its local backend and a capturing telemetry exporter.
  • Live AWS behavior was not revalidated; no cloud deployment or credentials were used.

Decisions not covered by the issue or the design note

  • The caller's type-only display message is exactly the supplied type.
  • An explicitly empty message is treated like an absent message for display, while the stored field remains empty.
  • All four null/empty type-and-message combinations display "Callback failed", as does an entirely absent Error. Whitespace handling is unchanged; no isBlank expansion.
  • No conformance tests or production telemetry code were changed.

Checklist

  • I have filled out every applicable section of the PR template.
  • I have tested this change locally.
  • Null-message, empty-message, and present-message tests distinguish each new branch.
  • Regression tests failed before the corresponding fix and passed after it.
  • Changed expectations are identified: in testCallbackFailureMessagePreservesPresentFields, the null/null and empty/empty rows now expect "Callback failed"; two mixed null/empty rows were added for the same fallback.

Testing

Unit Tests

On pristine main at 64339d4, the original CallbackExceptionTest regression set ran 11 cases with two failures: expected "RejectedError", got "RejectedError: null" or "RejectedError: ". Those cases pass with the fix.

The additional empty-ErrorObject fallback was separately checked against b17f2c4: the latest 13-case class had four assertion failures (expected "Callback failed", received null or empty). At latest head 6dcc03e, all 13 cases pass. Controls assert unchanged underlying error fields.

Integration Tests

Both new CallbackFailureMessageTest cases fail without the original fix and pass with it. They exercise an uncaught callback failure through the SDK and telemetry exporter, plus a caught failure followed by a durable wait and resume. They verify exported/root messages (including the name-keyed record shape), unchanged callback error and ID, and that a completed step executes only once.

Latest-head validation (6dcc03e):

  • mvn -pl telemetry-exporter -am test: 1,899 tests reported, 5 skipped, zero failures/errors (sdk: 1,598; sdk-testing: 53; telemetry-exporter: 248).
  • mvn spotless:check: passed across the full reactor. Original patch also passed spotless:apply/check.
  • git diff --check: passed.

Validation used Java 17 and Maven 3.9.11. This container cannot dynamically attach Mockito, so test runs supplied the existing Mockito 5.24.0 JAR at JVM startup via JAVA_TOOL_OPTIONS=-javaagent:/path/to/mockito-core-5.24.0.jar; no POM/dependency changes were made. The complete unrelated module/cloud/conformance suites were not run. Local validation does not replace maintainer review or approval of fork workflows.

Examples

Not applicable; no public usage change.

AI assistance

Codex generated the original and follow-up code/tests, inspected the relevant implementation and contribution rules, and executed the before/after and full related-module checks. This is AI-assisted work, not a claim of manual authorship.

Return the supplied error type when the callback error message is absent or empty. Preserve the original ErrorObject and cover telemetry plus durable replay.

AI assistance: Codex generated the fix/tests and ran local verification.
@YingzuoLiu
YingzuoLiu requested a review from a team October 30, 2026 05:34
@YingzuoLiu
YingzuoLiu temporarily deployed to ai-pr-review-runtime October 30, 2026 05:35 — with GitHub Actions Inactive
@github-actions

Copy link
Copy Markdown
Contributor

Codex AI review

One approach finding at telemetry-exporter/src/test/java/software/amazon/lambda/durable/telemetry/CallbackFailureMessageTest.java:24. No correctness defect confirmed. Tests were not executed during this read-only review.

Reviewed commit 6dcc03ed6920d9781e1a7858349bddd66559221a. Workflow run

@zhongkechen
zhongkechen merged commit 1a50140 into aws:main Oct 30, 2026
3 checks passed
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.

[Bug]: Type-only callback errors append ': null' to caller and telemetry messages

2 participants