UshakovVasilii / gnome-shell-extension-freon

Shows CPU temperature, disk temperature, video card temperature (NVIDIA/Catalyst/Bumblebee&NVIDIA), voltage and fan RPM
https://extensions.gnome.org/extension/841/freon
GNU General Public License v2.0
431 stars 77 forks source link

nvme-cli sometimes returns malformed JSON #215

Open BathoryPeter opened 3 years ago

BathoryPeter commented 3 years ago

When I select nvme-cli in settings, I get the following message in journal:

JS ERROR: SyntaxError: JSON.parse: unexpected character at line 1 column 1 of the JSON data @ /home/bp/.local/share/gnome-shell/extensions/freon@UshakovVasilii_Github.yahoo.com/nvmecliUtil.js:7

If I'm right, the mentioned code executes nvme list -o json. Running this command in terminal returns:

bathory-laptop:~$ nvme list -o json                                 
Failed to open /dev/nvme0
{
  "Devices" : [
    {
      "NameSpace" : 1,
      "DevicePath" : "/dev/nvme0n1",
      "Firmware" : "        ",
      "Index" : 0,
      "ModelNumber" : "                                        ",
      "ProductName" : "Non-Volatile memory controller: KIOXIA Corporation Device 0x0001",
      "SerialNumber" : "                    ",
      "UsedBytes" : 0,
      "MaximumLBA" : 0,
      "PhysicalSize" : 0,
      "SectorSize" : 1
    }
  ]
}

Which cannot be parsed because the "Failed to" string.

BetterToAutomateTheWorld commented 3 years ago

Hi,

I'm encountering the same issue with nvme-cli on Debian 11, so can't have nvme temp on the freon extension right now :(

If I can help debugging, let me know

jonasmalacofilho commented 3 years ago

This would most likely be an issue in nvme-cli, as we already only parse stdout (which, logically, should only output JSON when -o json is passed).

@BathoryPeter, what version of nvme-cli are you running? Please also mention the distro and distro version.

@Darcidride is the malformed data exactly the same? I ask because Debian 11 packages nvme-cli 1.12, and on that version Failed to open <dev path> appears to already go to stderr.

By the way, it seems that particular check & error message have also been removed entirely in 1.15, which may also explain why I can't reproduce the problem on my system.


Related: #184

BathoryPeter commented 3 years ago

nvme version 1.14 on Ubuntu 21.10

You are right, redirecting stderr solves the problem. What do you think about ignoring error messages in this way: return JSON.parse(GLib.spawn_command_line_sync(${nvme} ${argv} -o json 2> /dev/null)[1].toString())

jonasmalacofilho commented 3 years ago

@BathoryPeter,

That change shouldn't be necessary (I'm not even sure it's valid), since we're already only trying to parse the output in stdout:^1

return JSON.parse(GLib.spawn_command_line_sync(`${nvme} ${argv} -o json`)[1].toString())
                                                                         ^^^
                                                                        stdout

Can you open Looking GlassAlt-F2, type lg then Return—and try running both command lines bellow:

GLib.spawn_command_line_sync("nvme list -o json")
GLib.spawn_command_line_sync("nvme list -o json 2>/dev/null")

Like I mentioned, I don't think the second one is valid, but I'm interested in seeing the output of both.

BathoryPeter commented 3 years ago

I see, thanks for linking the docs. Indeed, the output of the two command was the same.

I did some experiment by adding some logging to getNvmeData().

    global.log(`argv: "${argv}"`)
    global.log(GLib.spawn_command_line_sync(`${nvme} ${argv} -o json`)[1].toString());

I got the following messages:

Freon-Message: 21:16:50.394: argv: "list"

(gnome-shell:545385): Gjs-WARNING **: 21:16:50.401: Some code called array.toString() on a Uint8Array instance. Previously this would have interpreted the bytes of the array as a string, but that is nonstandard. In the future this will return the bytes as comma-separated digits. For the time being, the old behavior has been preserved, but please fix your code anyway to explicitly call ByteArray.toString(array).
(Note that array.toString() may have been called implicitly.)
0 getNvmeData() ["/home/bp/.local/share/gnome-shell/extensions/freon@UshakovVasilii_Github.yahoo.com/nvmecliUtil.js":8:74]
1 nvmecliUtil() ["/home/bp/.local/share/gnome-shell/extensions/freon@UshakovVasilii_Github.yahoo.com/nvmecliUtil.js":16:32]
2 _initDriveUtility() ["/home/bp/.local/share/gnome-shell/extensions/freon@UshakovVasilii_Github.yahoo.com/extension.js":214:36]
3 _init() ["/home/bp/.local/share/gnome-shell/extensions/freon@UshakovVasilii_Github.yahoo.com/extension.js":70:13]
4 enable() ["/home/bp/.local/share/gnome-shell/extensions/freon@UshakovVasilii_Github.yahoo.com/extension.js":675:16]
5 _callExtensionEnable() ["resource:///org/gnome/shell/ui/extensionSystem.js":168:31]
6 loadExtension() ["resource:///org/gnome/shell/ui/extensionSystem.js":351:25]
7 _loadExtensions/<() ["resource:///org/gnome/shell/ui/extensionSystem.js":597:17]
8 collectFromDatadirs() ["resource:///org/gnome/shell/misc/fileUtils.js":27:27]
9 _loadExtensions() ["resource:///org/gnome/shell/ui/extensionSystem.js":572:18]
10 _enableAllExtensions() ["resource:///org/gnome/shell/ui/extensionSystem.js":606:17]
11 _sessionUpdated() ["resource:///org/gnome/shell/ui/extensionSystem.js":637:17]
12 init() ["resource:///org/gnome/shell/ui/extensionSystem.js":57:13]
13 _initializeUI() ["resource:///org/gnome/shell/ui/main.js":278:21]
14 start() ["resource:///org/gnome/shell/ui/main.js":175:4]
15 <TOP LEVEL> ["<main>":1:46]

Freon-Message: 21:16:50.401: {
  "Devices" : [
    {
      "NameSpace" : 1,
      "DevicePath" : "/dev/nvme0n1",
      "Firmware" : "        ",
      "Index" : 0,
      "ModelNumber" : "                                        ",
      "ProductName" : "Non-Volatile memory controller: KIOXIA Corporation Device 0x0001",
      "SerialNumber" : "                    ",
      "UsedBytes" : 0,
      "MaximumLBA" : 0,
      "PhysicalSize" : 0,
      "SectorSize" : 1
    }
  ]
}

(gnome-shell:545385): Gjs-WARNING **: 21:16:50.408: Some code called array.toString() on a Uint8Array instance. Previously this would have interpreted the bytes of the array as a string, but that is nonstandard. In the future this will return the bytes as comma-separated digits. For the time being, the old behavior has been preserved, but please fix your code anyway to explicitly call ByteArray.toString(array).
(Note that array.toString() may have been called implicitly.)
0 getNvmeData() ["/home/bp/.local/share/gnome-shell/extensions/freon@UshakovVasilii_Github.yahoo.com/nvmecliUtil.js":9:81]
1 nvmecliUtil() ["/home/bp/.local/share/gnome-shell/extensions/freon@UshakovVasilii_Github.yahoo.com/nvmecliUtil.js":16:32]
2 _initDriveUtility() ["/home/bp/.local/share/gnome-shell/extensions/freon@UshakovVasilii_Github.yahoo.com/extension.js":214:36]
3 _init() ["/home/bp/.local/share/gnome-shell/extensions/freon@UshakovVasilii_Github.yahoo.com/extension.js":70:13]
4 enable() ["/home/bp/.local/share/gnome-shell/extensions/freon@UshakovVasilii_Github.yahoo.com/extension.js":675:16]
5 _callExtensionEnable() ["resource:///org/gnome/shell/ui/extensionSystem.js":168:31]
6 loadExtension() ["resource:///org/gnome/shell/ui/extensionSystem.js":351:25]
7 _loadExtensions/<() ["resource:///org/gnome/shell/ui/extensionSystem.js":597:17]
8 collectFromDatadirs() ["resource:///org/gnome/shell/misc/fileUtils.js":27:27]
9 _loadExtensions() ["resource:///org/gnome/shell/ui/extensionSystem.js":572:18]
10 _enableAllExtensions() ["resource:///org/gnome/shell/ui/extensionSystem.js":606:17]
11 _sessionUpdated() ["resource:///org/gnome/shell/ui/extensionSystem.js":637:17]
12 init() ["resource:///org/gnome/shell/ui/extensionSystem.js":57:13]
13 _initializeUI() ["resource:///org/gnome/shell/ui/main.js":278:21]
14 start() ["resource:///org/gnome/shell/ui/main.js":175:4]
15 <TOP LEVEL> ["<main>":1:46]

Freon-Message: 21:16:50.418: argv: "smart-log /dev/nvme0n1"
Freon-Message: 21:16:50.432: \u001b[1mUsage: nvme smart-log <device> [OPTIONS]\u001b[0m

\u001b[1mOptions:\u001b[0m

(gnome-shell:545385): Gjs-WARNING **: 21:16:50.438: JS ERROR: Extension freon@UshakovVasilii_Github.yahoo.com: SyntaxError: JSON.parse: unexpected character at line 1 column 1 of the JSON data @ /home/bp/.local/share/gnome-shell/extensions/freon@UshakovVasilii_Github.yahoo.com/nvmecliUtil.js:9

This is because smart-log requires sudo, but nvme-cli spams the stdout with non json error message. Maybe we can check the exit status before trying to parse the output.

jonasmalacofilho commented 3 years ago

This is because smart-log requires sudo, but nvme-cli spams the stdout with non json error message.

Oh, that makes sense.

Maybe we can check the exit status before trying to parse the output.

That would be better, yes. It will probably still result in an exception, but a more specific one that's easier for the end user to interpret (and end up at #184).

By the way, with Linux 5.11 (based on knowing that you're running Ubuntu 20.11), you shouldn't really need nvme-cli: the NVMe driver already has support for hwmon integration (since Linux 5.5). While that's optional, I imagine it's probably enabled on Ubuntu. Or, at least (and IMHO), it can/should be.

BathoryPeter commented 3 years ago

A controlled exception would be much better. A few weeks ago I tried to set the SSD temp display in preferences, without success. At next login Freon didn't want to load. I tried to reinstall, update, downgrade and a lot of other magic, and ended up in dconf editor, where I deleted drive-utility key, which solved the problem (but I still didn't have SSD temp). That's why I opened this issue.

Does Freon use hwmon? I can't find it in the code. Seeing /sys/class/hwmon/hwmon3/temp1_input seems to show the actual NVMe temp, but Freon wont display it in any way.

jonasmalacofilho commented 3 years ago

Does Freon use hwmon?

Yes, that's arguably the main source Freon gets its temperatures from. And the only one enabled by default.

IIRC, Freon uses the output from sensors -j, so you might want to check that (or its more human readable alternative).

And since Freon simply uses sensors, any adjustments or renaming of sensors through sensors.conf will also be taken into account.

BathoryPeter commented 3 years ago

Oh, I get it! So on a regular install, selecting None in preferences must be enough. Then the preferences window is a bit misleading. I thought selecting None means HDD temperature querrying is switched off.

Running sensors I get this:

nvme-pci-0200
Adapter: PCI adapter
Composite:    +26.9°C  (low  = -273.1°C, high = +82.8°C)
                       (crit = +86.8°C)
Sensor 1:     +26.9°C  (low  = -273.1°C, high = +65261.8°C)

And really, Freon showed me Composite and Sensor 1 all the time. What bad names!