Skip to content

fix(OPENFRAM-002-9): CU-86akdyq05 MachineHostnameListener leaves the message unacked on any parse or extraction error, risking infinite redelivery #2079

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
flamingo wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

fix(OPENFRAM-002-9): CU-86akdyq05 MachineHostnameListener leaves the message unacked on any parse or extraction error, risking infinite redelivery #2079

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions ...rame-client-core/src/main/java/com/openframe/client/listener/MachineHostnameListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.openframe.client.listener;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.openframe.client.service.MachineHostnameService;
import com.openframe.client.service.NatsTopicMachineIdExtractor;
import com.openframe.data.nats.listener.AbstractJetStreamPushListener;
Expand Down Expand Up @@ -81,6 +82,9 @@ protected void handleMessage(Message message) {

message.ack();
log.debug("Hostname update processed successfully and acked");
} catch (JsonProcessingException | IllegalArgumentException e) {
log.error("Non-retryable error processing hostname update, acking to avoid redelivery loop: {}", messagePayload, e);
message.ack();
} catch (Exception e) {

@flamingo flamingo Bot Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🟠 MachineHostnameListener leaves the message unacked on any parse or extraction error, risking infinite redelivery

In handleMessage, added a new catch (JsonProcessingException | IllegalArgumentException e) block (before the generic catch (Exception e)) that logs and calls message.ack() instead of leaving the message unacked. This targets the specific permanent-failure cases called out in the finding: malformed JSON (JsonProcessingException from objectMapper.readValue) and malformed subject (assuming machineIdExtractor.extract throws IllegalArgumentException on an unparseable subject, which is a common convention but not verified against that class's actual implementation in this file). Also added the com.fasterxml.jackson.core.JsonProcessingException import. The remaining generic Exception catch still leaves messages unacked for genuinely transient errors (e.g. machineHostnameService.updateHostname failures), preserving existing redelivery behavior for those. A complete fix would require confirming the actual exception type(s) thrown by NatsTopicMachineIdExtractor.extract for malformed subjects and verifying updateHostname's failure modes to ensure no permanent-failure exceptions leak into the unbounded-retry path.

🤖 Prompt for AI agents 
In openframe-client-core/src/main/java/com/openframe/client/listener/MachineHostnameListener.java around line 84, review and complete this code-review fix: MachineHostnameListener leaves the message unacked on any parse or extraction error, risking infinite redelivery.
What the draft fix changed: In `handleMessage`, added a new `catch (JsonProcessingException | IllegalArgumentException e)` block (before the generic `catch (Exception e)`) that logs and calls `message.ack()` instead of leaving the message unacked. This targets the specific permanent-failure cases called out in the finding: malformed JSON (`JsonProcessingException` from `objectMapper.readValue`) and malformed subject (assuming `machineIdExtractor.extract` throws `IllegalArgumentException` on an unparseable subject, which is a common convention but not verified against that class's actual implementation in this file). Also added the `com.fasterxml.jackson.core.JsonProcessingException` import. The remaining generic `Exception` catch still leaves messages unacked for genuinely transient errors (e.g. `machineHostnameService.updateHostname` failures), preserving existing redelivery behavior for those. A complete fix would require confirming the actual exception type(s) thrown by `NatsTopicMachineIdExtractor.extract` for malformed subjects and verifying `updateHostname`'s failure modes to ensure no permanent-failure exceptions leak into the unbounded-retry path.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

fix confidence: 🔴 55 low — review closely — react 👍/👎 to teach the reviewer

log.error("Unexpected error processing hostname update: {}", messagePayload, e);
// Don't ack the message and let it be redelivered
Expand Down