Skip to content

Improve error handling for unsupported server commands#373

Merged
ravjotbrar merged 16 commits into
mainfrom
fix/monitor-error
Jun 24, 2026
Merged

Improve error handling for unsupported server commands#373
ravjotbrar merged 16 commits into
mainfrom
fix/monitor-error

Conversation

@ravjotbrar

@ravjotbrar ravjotbrar commented Jun 19, 2026

Copy link
Copy Markdown
Collaborator

Description

Summary

When connecting to a server that doesn't support certain commands (e.g., ElastiCache
Serverless), Valkey Admin previously showed confusing errors, blank UI states, or
falsely reported features as active. This PR surfaces clear error messages to the user
and adds a connection-time warning for serverless caches.

Problem

  • Monitor showed "Active" even when the MONITOR command was rejected by the server
  • Command Logs crashed with "Cannot read properties of null" when COMMANDLOG was
    unsupported
  • No indication to the user that they were connecting to a limited environment

Changes

  • Monitor: The metrics server now waits for the first MONITOR cycle to succeed before
    reporting "running." If the command is rejected, the error propagates to the frontend
    and displays in the Activity view.
  • Command Logs: The metrics server now stores startup failures and returns a proper
    error response instead of crashing when the collector never initialized. Fixed a key
    mismatch between the request type and collector name that caused the error check to be
    bypassed.
  • Serverless detection: On connection, if the hostname contains .serverless., a toast
    warns the user which features are unavailable.
  • Memory Usage: When MEMORY USAGE is unavailable (e.g., blocked on serverless), key sizes now display "—" instead of "0 B". The total memory calculation and distribution chart skip keys with unknown sizes rather than counting them as zero.
  • General: Unknown command errors in the monitor stream now terminate the retry loop
    instead of retrying indefinitely.

Signed-off-by: ravjotb <ravjot.brar@improving.com>
Signed-off-by: ravjotb <ravjot.brar@improving.com>
Signed-off-by: ravjotb <ravjot.brar@improving.com>
Signed-off-by: ravjotb <ravjot.brar@improving.com>
Signed-off-by: ravjotb <ravjot.brar@improving.com>
Signed-off-by: ravjotb <ravjot.brar@improving.com>
Signed-off-by: ravjotb <ravjot.brar@improving.com>
Signed-off-by: ravjotb <ravjot.brar@improving.com>
Signed-off-by: ravjotb <ravjot.brar@improving.com>
Signed-off-by: ravjotb <ravjot.brar@improving.com>
Signed-off-by: ravjotb <ravjot.brar@improving.com>
Signed-off-by: ravjotb <ravjot.brar@improving.com>
Signed-off-by: ravjotb <ravjot.brar@improving.com>
ArgusLi
ArgusLi previously approved these changes Jun 22, 2026

@ArgusLi ArgusLi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just a small question. I like the way init-collectors works way more now.

Comment thread apps/frontend/src/components/ui/donut-chart.tsx
Signed-off-by: ravjotb <ravjot.brar@improving.com>
Comment thread apps/metrics/src/effects/monitor-stream.js Outdated
},
})

monitorStopper = async () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does this split from a singular updateCollectorMeta to 2 updateCollectorMetas have any upstream impact? Is there a reason why this needed to be split?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good question. I split runMonitorOnce into two separate functions because I needed a way to return immediately if the monitor command wasn't supported. Before the code would need to await the entire duration of monitor for runMonitorOnce to return. Now we can await connectMonitor for an immediate result.

ravjotbrar and others added 2 commits June 24, 2026 11:31
Signed-off-by: Ravjot Brar <83892020+ravjotbrar@users.noreply.github.com>
Signed-off-by: ravjotb <ravjot.brar@improving.com>
@ravjotbrar ravjotbrar merged commit 6d0e8e0 into main Jun 24, 2026
8 checks passed
@ArgusLi ArgusLi deleted the fix/monitor-error branch June 25, 2026 17:03
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