Skip to content

FINERACT-2716: Log SMS campaign preview JSON parsing failure instead of silently swallowing it - #6190

Open
AshharAhmadKhan wants to merge 1 commit into
apache:developfrom
AshharAhmadKhan:FINERACT-2716-log-sms-campaign-preview-parse-failure
Open

FINERACT-2716: Log SMS campaign preview JSON parsing failure instead of silently swallowing it#6190
AshharAhmadKhan wants to merge 1 commit into
apache:developfrom
AshharAhmadKhan:FINERACT-2716-log-sms-campaign-preview-parse-failure

Conversation

@AshharAhmadKhan

Copy link
Copy Markdown
Contributor

JIRA
https://issues.apache.org/jira/browse/FINERACT-2716

Problem
The previewMessage-equivalent method in SmsCampaignWritePlatformServiceJpaImpl
caught IOException from JSON parsing and did nothing with it. No log, no
rethrow. The failure disappeared silently.

Fix
Replaced the // TODO throw something here with a log.error call. This was the
last remaining instance of this pattern in the class - every other
catch (IOException ...) block here already logs and continues.

This is a direct follow-up to FINERACT-2707, which fixed the equivalent issue
in EmailCampaignWritePlatformCommandHandlerImpl, and to the note left in that
PR flagging this exact method as the one remaining spot.

I also grepped the full codebase for "TODO throw something here" and for any
other silent catch (IOException e) {} blocks - this was the only remaining
occurrence; everything else already logs.

No functional change otherwise.

@AshharAhmadKhan

Copy link
Copy Markdown
Contributor Author

hey @adamsaghy please trigger checks whenever possible, thanks!

@AshharAhmadKhan

Copy link
Copy Markdown
Contributor Author

hey @adamsaghy , failures look flaky, can you please retrigger those checks? thanks a lot!

@AshharAhmadKhan

Copy link
Copy Markdown
Contributor Author

Sorry for the ping @adamsaghy . I think this is ready for review !

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