Skip to content

fix: json parse crashes in monitor model with safe parsing#6767

Merged
CommanderStorm merged 6 commits intolouislam:masterfrom
dharunashokkumar:fix-json-parse-crash
Jan 19, 2026
Merged

fix: json parse crashes in monitor model with safe parsing#6767
CommanderStorm merged 6 commits intolouislam:masterfrom
dharunashokkumar:fix-json-parse-crash

Conversation

@dharunashokkumar
Copy link
Copy Markdown
Contributor

fixes json parse crashes when invalid json is stored in database for kafka and rabbitmq monitors.

added safe json parsing with fallback to default values instead of crashing.

Copilot AI review requested due to automatic review settings January 19, 2026 15:09
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes crashes caused by invalid JSON in the database for Kafka and RabbitMQ monitor configurations by introducing safe JSON parsing with fallback to default values.

Changes:

  • Added a safeJsonParse static method to handle JSON parsing errors gracefully
  • Replaced all direct JSON.parse() calls for Kafka and RabbitMQ monitor fields with safe parsing
  • Implemented error logging when JSON parsing fails

rabbitmqNodes: JSON.parse(this.rabbitmqNodes),
conditions: JSON.parse(this.conditions),
rabbitmqNodes: Monitor.safeJsonParse(this.rabbitmqNodes, [], "rabbitmqNodes"),
conditions: Monitor.safeJsonParse(this.conditions, null, "conditions"),
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

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

Using null as the default value for conditions may be inconsistent with other fields that use empty arrays or objects. Consider using an empty array [] or empty object {} as the default depending on the expected data structure for conditions, to prevent potential null reference issues in code that consumes this field.

Suggested change
conditions: Monitor.safeJsonParse(this.conditions, null, "conditions"),
conditions: Monitor.safeJsonParse(this.conditions, [], "conditions"),

Copilot uses AI. Check for mistakes.
@dharunashokkumar dharunashokkumar changed the title fix json parse crashes in monitor model with safe parsing fix: json parse crashes in monitor model with safe parsing Jan 19, 2026
@CommanderStorm
Copy link
Copy Markdown
Collaborator

CommanderStorm commented Jan 19, 2026

how did the invalid json end up in the db in the first place?

I think a better place to fix this is Monitor.validatte => fix it before we have garbage in the db

@CommanderStorm CommanderStorm added the pr:please address review comments this PR needs a bit more work to be mergable label Jan 19, 2026
@CommanderStorm CommanderStorm marked this pull request as draft January 19, 2026 15:34
@dharunashokkumar dharunashokkumar marked this pull request as ready for review January 19, 2026 17:59
@github-actions github-actions bot added pr:needs review this PR needs a review by maintainers or other community members and removed pr:please address review comments this PR needs a bit more work to be mergable labels Jan 19, 2026
Copy link
Copy Markdown
Collaborator

@CommanderStorm CommanderStorm left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@CommanderStorm CommanderStorm merged commit f8d494a into louislam:master Jan 19, 2026
24 checks passed
@github-actions
Copy link
Copy Markdown
Contributor

@dharunashokkumar congrats on your first contribution to Uptime Kuma! 🐻
We hope you enjoy contributing to our project and look forward to seeing more of your work in the future! If you want to see your contribution in action, please see our nightly builds here.

@dharunashokkumar dharunashokkumar deleted the fix-json-parse-crash branch January 20, 2026 04:26
@CommanderStorm CommanderStorm added this to the 2.1.0 milestone Jan 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:needs review this PR needs a review by maintainers or other community members

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants