Skip to content

fix: add error handling for K8s API, Redis pipeline, demos, and config validation#10

Open
Flegma wants to merge 3 commits intomainfrom
audit/410-error-handling
Open

fix: add error handling for K8s API, Redis pipeline, demos, and config validation#10
Flegma wants to merge 3 commits intomainfrom
audit/410-error-handling

Conversation

@Flegma
Copy link
Copy Markdown
Contributor

@Flegma Flegma commented Apr 8, 2026

Summary

  • Add try/catch with logging to getNode() K8s API call and handle undefined from getNodeMetrics()
  • Replace fire-and-forget void redis.multi().exec() with await + try/catch + error logging in WebRTC latency storage
  • Replace deprecated rmdirSync(path, {recursive:true}) with rmSync(path, {recursive:true}) and add existsSync guards before unlinkSync in demos service
  • Add NaN validation after parseInt calls in API config to fail fast on missing env vars
  • Add NaN check on CPU governor path parsing with continue if invalid
  • Sanitize NIC name before interpolating into bash command to prevent shell injection

Addresses 5stackgg/5stack-panel#410 and 5stackgg/5stack-panel#411

Test plan

  • Verify connector starts correctly with valid env vars
  • Verify connector fails fast with clear error on missing/invalid port env vars
  • Verify demo upload handles missing files gracefully
  • Verify latency test results are still stored in Redis

Flegma added 2 commits April 8, 2026 14:02
- NetworkService: track IP check interval and spawned bash processes,
  add OnApplicationShutdown to clear interval and kill processes,
  kill monitor on error/stderr, remove process.on(SIGTERM) handler
- Throttle: store setInterval ID, add destroy() method to clear it
- RconService: only store connection after successful auth, add timeout
  to rcon.send() calls, clear connection timer after successful connect
- RedisManagerService: track health check intervals, add
  OnApplicationShutdown to clear intervals and disconnect
}

const metrics = await this.metricsClient.getNodeMetrics();

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.

undo t his.k there is no value to set a const

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.

Reverted -- put the .find() back inline in the return object.

const safeName = nic.replace(/[^a-zA-Z0-9_.-]/g, "");

const monitor = spawn("bash", [
"-c",
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.

im not confident in merging this.

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.

Removed the NIC name sanitization entirely.

done`,
]);

process.on(process.env.DEV ? "SIGUSR2" : "SIGTERM", () => {
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.

this is required for local dev.

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.

Restored the process.on(DEV ? 'SIGUSR2' : 'SIGTERM') signal handler.

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