Skip to content

fix(gateway): recover dead-lettered request logs - #1037

Draft
sbalabanov wants to merge 2 commits into
mainfrom
wt/sqc010
Draft

sbalabanov wants to merge 2 commits into
mainfrom
wt/sqc010

Conversation

@sbalabanov

Copy link
Copy Markdown
Contributor

Summary

A terminal request log that exhausts primary retries can leave the public status permanently stale. Replay the gateway-owned final log backlog through the existing materializer, retain malformed payloads and envelopes, and clean up both consumers on partial startup or shutdown.

Test Plan

  1. Seed a terminal log in log_dlq before the gateway starts, while its receipt summary is unavailable.
  2. Observe recovery retrying without acknowledgment, then restore the summary without publishing another event.
  3. TestFinalLogBacklogRecovery verifies the repaired terminal summary, URI mapping, List receipt, and retained history. TestLogConsumers_RetainFailedRecovery, TestServeGateway_ConsumerCleanup, and TestConsumer_ProcessDelivery_InvalidTenantMetadata cover retention and lifecycle failures.

Revert Plan

Revert this change and leave the log_dlq backlog intact.

Issues

Summary:
A terminal request log that exhausts primary retries can leave the public status permanently stale. Replay the gateway-owned final log backlog through the existing materializer, retain malformed payloads and envelopes, and clean up both consumers on partial startup or shutdown.

Test Plan:
1. Seed a terminal log in log_dlq before the gateway starts, while its receipt summary is unavailable.
2. Observe recovery retrying without acknowledgment, then restore the summary without publishing another event.
3. TestFinalLogBacklogRecovery verifies the repaired terminal summary, URI mapping, List receipt, and retained history. TestLogConsumers_RetainFailedRecovery, TestServeGateway_ConsumerCleanup, and TestConsumer_ProcessDelivery_InvalidTenantMetadata cover retention and lifecycle failures.

Revert Plan:
Revert this change and leave the log_dlq backlog intact.
@sbalabanov

Copy link
Copy Markdown
Contributor Author

[independent-review] Should fix — shutdown failures can be hidden (95% confidence).

In service/submitqueue/gateway/server/main.go:381, joining context.Canceled with a consumer-stop error keeps errors.Is(err, context.Canceled) true. Repro: SIGTERM wins the server select, then an in-flight controller exceeds the consumer shutdown timeout. main takes its ordinary signal branch instead of reporting the cleanup failure (the previous code replaced expected cancellation when cleanup failed).

Smallest correction: return the stop error instead of expected cancellation when cleanup fails; still join genuine listener/server failures and keep stopping both consumers. Extend the existing cleanup table with cancellation + stop error; assert both consumers stop, the error contains stopErr, and it does not contain context.Canceled. No new test suite is needed.

All four affected package tests, including fresh Docker integration, passed independently. The lifecycle/policy refactoring already removes duplicated branching; no additional safe complexity reduction was established.

Generated with the repository's .agents/skills/autoreview workflow; finding rechecked against PR #1037 at 46fd1258d94373f55a845035a7c453a2367a68e7.

Keep expected signal cancellation from masking a failed consumer stop. Preserve joined cleanup errors for listener and server failures, and continue stopping both consumers.

Test Plan:
Extend TestServeGateway_ConsumerCleanup with cancellation plus a stop failure; verify both consumers stop, the stop error survives, and context.Canceled does not.

Revert Plan:
Revert this follow-up commit.
@sbalabanov

Copy link
Copy Markdown
Contributor Author

[agent-response SQC-010] Addressed #1037 (comment) in commit 2669cfb. A consumer-stop failure now replaces expected signal cancellation; genuine listener/server errors still join cleanup failures, and both consumers are stopped. Extended TestServeGateway_ConsumerCleanup with cancellation plus a stop failure, verifying stopErr is retained and context.Canceled is absent. The focused Bazel package test and formatting/lint/tidy/Gazelle checks passed locally. The PR remains a draft; skipped code-CI checks are not counted as passes.

This branch has not been deployed

No deployments
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.

2 participants