Repository: louislam/uptime-kuma
File: server/monitor-types/tailscale-ping.js (JavaScript)

Code before:
// In cases where offline peers are not pingable, how should he handle timeout messages? Do we break out of the "ping (ip) timed out" loop and let the server re-call the check function? or do we include the "timed out" string in the expected error messages line and throw the error? (along with "no matching peer" and "is local Tailscale IP")

const { MonitorType } = require("./monitor-type");
const { UP } = require("../../src/util");

// child_process
const exec = require('child_process').exec;

// cases accounted for
// 1 calling tailscale ping to local
// CMD: tailscale ping 100.100.100.1
// 100.100.100.1 is local Tailscale IP
// 2 the normal scenario
// CMD: tailscale ping 100.100.100.2
// pong from (hostname) (100.100.100.2) via [2001:1245:1234:1234::abc]:12345 in 123ms
// 3 ping a nonexistent IP or inaccessible node
// CMD: tailscale ping 100.100.100.3
// no matching peer
// 4 ping a peer that you cannot make a outgoing connection to
// CMD: tailscale ping 100.100.100.4
// multiple 'pong from' lines (pong from hostname (100.100.100.4) via DERP(tok) in 123ms) then ends with a 'direct connection not established' line
// We only care about the first 'pong from' line for the purpose of checking uptime. (this is a good example use of "tailscale ping" becasue traditional ICMP pings would not be able to check the uptime of peers if outgoing connections to that specified peer are blocked)
// 5 pinging a offline peer
// CMD: tailscale ping 100.100.100.5
// multiple 'timed out' lines (ping "100.100.100.5" timed out) then ends with a 'no reply' line
// This needs to be worked on

class TailscalePing extends MonitorType {

    name = "tailscale-ping";

    async check(monitor, heartbeat) {
        try {
            let tailscaleOutput = await this.runTailscalePing(monitor.url);
            this.parseTailscaleOutput(tailscaleOutput, heartbeat);
        } catch (err) {
            // trigger log function somewhere to display a notification or alert to the user (but how?)
            throw new Error(`Error checking Tailscale ping: ${err}`);
        }
    }

// runTailscalePing is now marked as async
    async runTailscalePing(url) {
        let cmd = `tailscale ping ${url}`;
        return new Promise((resolve, reject) => {
            //set a timeout here just incase tailscale ping hangs
            exec(cmd, {timeout: 10000}, (error, stdout, stderr) => {
                // Also to trigger a function here of error.message or stderr in the web gui 
                // we may need to handle more cases if tailscale reports an error that isn't necessarily an error (such as not-logged in or DERP health-related issues) 
                if (error) {
                    reject(`Execution error: ${error.message}`);

Review comment (suggestion):
I am going to be controversioal:
This comment is really hard to parse and in general, large comments explaining functionality rott realy fast.
Could you integrate this into the code where you account for these cases if they are not self-explanatory?

See https://www.youtube.com/watch?v=Bf7vDBBOBUA for a better explaination of this argument

Code after:
const { UP, log } = require("../../src/util");
const exec = require("child_process").exec;

/**
 * A TailscalePing class extends the MonitorType.
 * It runs Tailscale ping to monitor the status of a specific node.
 */
class TailscalePing extends MonitorType {

    name = "tailscale-ping";

    /**
     * Checks the ping status of the URL associated with the monitor.
     * It then parses the Tailscale ping command output to update the heatrbeat.
     *
     * @param {Object} monitor - The monitor object associated with the check.
     * @param {Object} heartbeat - The heartbeat object to update.
     * @throws Will throw an error if checking Tailscale ping encounters any error
     */
    async check(monitor, heartbeat) {
        try {
            let tailscaleOutput = await this.runTailscalePing(monitor.hostname, monitor.interval);
            this.parseTailscaleOutput(tailscaleOutput, heartbeat);
        } catch (err) {
            log.debug("Tailscale", err);
            // trigger log function somewhere to display a notification or alert to the user (but how?)
            throw new Error(`Error checking Tailscale ping: ${err}`);
        }
    }

    /**
     * Runs the Tailscale ping command to the given URL.
     *
     * @param {string} hostname - The hostname to ping.
     * @returns {Promise<string>} - A Promise that resolves to the output of the Tailscale ping command
     * @throws Will throw an error if the command execution encounters any error.
     */
    async runTailscalePing(hostname, interval) {
        let cmd = `tailscale ping ${hostname}`;

        log.debug("Tailscale", cmd);

        return new Promise((resolve, reject) => {
            let timeout = interval * 1000 * 0.8;
            exec(cmd, { timeout: timeout }, (error, stdout, stderr) => {
                // we may need to handle more cases if tailscale reports an error that isn't necessarily an error (such as not-logged in or DERP health-related issues)
                if (error) {
                    reject(`Execution error: ${error.message}`);
                    return;
                }
                if (stderr) {