Skip to content

fix(plugin): pair end hook when result delivery fails - #739

Closed
zhongkechen wants to merge 4 commits into
mainfrom
fix/result-delivery-end-hook
Closed

zhongkechen wants to merge 4 commits into
mainfrom
fix/result-delivery-end-hook

Conversation

@zhongkechen

@zhongkechen zhongkechen commented Sep 25, 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

Extracted as an independent fix from #721.

Description

The successful-handler path serialized the result and checkpointed an oversized payload before firing onInvocationEnd. If serialization or the checkpoint failed, plugins received a start hook without the matching end hook. That leaves telemetry unfinished: buffered exporters are not flushed and OTel spans are not ended.

Capture result-delivery failures, unwrap CompletionException from a failed checkpoint, fire onInvocationEnd with RETRYING, then rethrow the original failure. The result passed to the hook remains null because it was not delivered. No public API changes.

Demo/Screenshots

Not applicable.

Checklist

  • I have filled out every section of the PR template
  • I have thoroughly tested this change

Testing

Unit Tests

Added a real oversized-result checkpoint failure test through DurableExecutor and the checkpoint batcher. It verifies the EXECUTION/SUCCEED request, exactly one paired start/end hook, RETRYING, a null undelivered result, and the same original exception at the hook and caller.

DurableExecutionTest: 11 tests passed. Isolated mutation checks confirm the new test fails if either the end-hook fix or exception unwrapping is removed.

Integration Tests

The local-runner serialization failure regression remains covered by PluginIntegrationTest (34 tests passed). The simulated wrapped-SerDes test was replaced by the actual checkpoint test above. Formatting checks passed.

Examples

Not applicable.

@zhongkechen
zhongkechen requested a review from a team September 25, 2026 21:58
@zhongkechen
zhongkechen deployed to ai-pr-review-runtime September 25, 2026 21:58 — with GitHub Actions Active
@zhongkechen
zhongkechen deployed to ai-pr-review-runtime September 25, 2026 22:15 — with GitHub Actions Active
@github-actions

This comment has been minimized.

@zhongkechen
zhongkechen deployed to ai-pr-review-runtime September 25, 2026 23:04 — with GitHub Actions Active
@github-actions

Copy link
Copy Markdown
Contributor

Codex AI review

No actionable findings. Residual risk is limited to uncommon throwable variants not explicitly covered by the added serialization and checkpoint-failure tests.

Reviewed commit 77d67a9a5d7b8cdc059f3201c5c51e876cb9dea6. Workflow run

@zhongkechen
zhongkechen deleted the fix/result-delivery-end-hook branch September 29, 2026 17:31

This branch was successfully deployed

1 active deployment
ai-pr-review-runtime — 77d67a9a Deployed Sep 25, 2026 by zhongkechen via ai-pr-review / Codex review / Generate Codex review #1123
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