Enhance JSON response parsing, prevent error leak

[bazzani]

If the JSON response from speedtest does not contain a property with the value - "type": "result", then a problem has occurred and the script should not attempt to extract the values from the JSON object or perform a database save.

Currently, a non-user friendly error message is shown when a problem occurs, but we should prevent this script error and show a more friendly message.

Another side effect of not handling the error is that the script will exit with a non-zero status, in effect killing the container.

Network problems may be intermittent, and if the container is running the speed tests in a loop, we want to be able to handle those intermittent problems without the container dying.

Before this change:

docker run -e SPEEDTEST_SERVER_ID=123 -e LOOP=true -e LOOP_DELAY=5 barryevans80/speedtest:wip
Running speedtest forever... ♾️

Running a Speed Test with Server ID 123...
{"type":"log","timestamp":"2024-06-17T13:34:56Z","message":"Configuration - No servers defined (NoServersException)","level":"error"}
./speedtest.sh: 33: arithmetic expression: expecting primary: "  / 125000 "

After this change:

docker run -e SPEEDTEST_SERVER_ID=123 -e LOOP=true -e LOOP_DELAY=5 barryevans80/speedtest:wip
Running speedtest forever... ♾️

Running a Speed Test with Server ID 123...
{"type":"log","timestamp":"2024-06-17T13:33:29Z","message":"Configuration - No servers defined (NoServersException)","level":"error"}
[error] Unable to retrieve JSON results from speedtest, please see previous log message
Running next test in 5s...

Running a Speed Test with Server ID 123...
{"type":"log","timestamp":"2024-06-17T13:33:36Z","message":"Configuration - No servers defined (NoServersException)","level":"error"}
[error] Unable to retrieve JSON results from speedtest, please see previous log message
Running next test in 5s...

---

The validation required to make the script more robust is achieved by using a simple jq select function:

• echo "$JSON" | jq 'select(.type == "result")'

See more about the jq select function here:

• https://jqlang.github.io/jq/manual/#select

---

Some shellcheck fixes have been applied:

1. $/${} is unnecessary on arithmetic variables. - https://github.com/koalaman/shellcheck/wiki/SC2004
2. Double quote to prevent globbing and word splitting. - https://github.com/koalaman/shellcheck/wiki/SC2086

---

Sample successful json response

{
  "type": "result",
  "timestamp": "2024-06-17T05:27:00Z",
  "ping": {
    "jitter": 0.852,
    "latency": 288.814,
    "low": 287.153,
    "high": 289.37
  },
  "download": {
    "bandwidth": 1591349,
    "bytes": 22220160,
    "elapsed": 15010,
    "latency": {
      "iqm": 286.068,
      "low": 282.946,
      "high": 293.54,
      "jitter": 1.568
    }
  },
  "upload": {
    "bandwidth": 38988010,
    "bytes": 493973670,
    "elapsed": 15005,
    "latency": {
      "iqm": 295.051,
      "low": 286.54,
      "high": 379.607,
      "jitter": 7.626
    }
  },
  ...
}

Sample failed json response

{
  "type": "log",
  "timestamp": "2024-06-17T05:25:48Z",
  "message": "Configuration - No servers defined (NoServersException)",
  "level": "error"
}

[bazzani]

I think this other change was introduced to help with error handling - 1f3c8db8 - but it relies on the container failing and being restarted (the logic is in the docker-compose service config)

• #10

I propose that a cleaner way to solve the problem is to have some JSON parsing inside the shell script.

Please let me know if you like this new change @robinmanuelthiel.

[bazzani]

Hi @robinmanuelthiel would you be interested in reviewing this enhancement?