report write errors if sqlite can not be created

[drewwells]

Resolve relative paths and attempt create before passing to sqlite

#12

[drewwells]

The error was the path I was writing to was read only to the user. The imported library may have a wat to surface the underlying write error. This error was a little vague

Error setup sqlite SqliteFailure(Error { code: CannotOpen, extended_code: 14 }, Some("unable to open database file: speedtest.db"))

[SudoDios]

@drewwells
Thank you 🌹.
Very good, but :

1. No need to create the file, because sqlite does it itself.
   We just need to create the directories std::fs::create_dir_all
2. Remove the prints (only write the log when setup encounters a problem).

[drewwells]

Okay I dug into the library a little bit (asking chatgpt) and came up with this. I wanted to get specific file i/o errors when the config was missing and if the sqlite db was not-writeable. This was the case when I first started using speedtest-rust with my systemd setup

DEMO

sqlite database is not writeable

speedtest[3856781]: [2024-11-27 09:29:43.712 ERROR src/main.rs:59] SQLite permission error: SqliteFailure(Error { code: CannotOpen, extended_code: 14 }, Some("unable to open database file: /var/lib/speedtest/speedtest.db"))
systemd[1]: speedtest.service: Main process exited, code=exited, status=1/FAILURE

config file was missing or directory not readable

speedtest[3851365]: [2024-11-27 09:26:52.884 ERROR src/main.rs:28] Could not load config from speedtest-settings.toml: No such file or directory (os error 2)

[SudoDios]

ChatGPT doesn't work very well ☺️.
But a few points:

1. There is still error code 14, when the directory for the database does not exist.
2. Where the directory is only readable or writable, it should be stated in the error (easier to understand).
3. There is no need to write the full source path.

[drewwells]

Looks okay to me, I assume people will figure out the directory doesn't exist based on the path being reported in the error.

[2024-11-28 06:00:06.043 INFO src/config/mod.rs:128] Configs initialized file : configs.toml
[2024-11-28 06:00:06.043 ERROR src/main.rs:56] Error setup sqlite SqliteFailure(Error { code: CannotOpen, extended_code: 14 }, Some("unable to open database file: /home/src/github.com/librespeed/speedtest-rust/boom/speedtest.db"))

What error message were you expecting? I think full path is helpful to debug missing directories. Especially since config doesn't define a root path so they're subject to relative pathing from dockerfile or systemd. I wouldn't be opposed to adding a mkdir in rust to attempt to create a root directory and stateful directories like sqlite

[SudoDios]

I mean, you don't need to show rec.file().unwrap_or("unknown") and rec.line().unwrap_or(0) in the log.
Step to show error :

• db directory exist
  • only readable or writable -> show error to user with full db path
  • continue;
• db directory not exist
  • mkdir
    • failure -> show error to user with full db path
    • success -> continue;

As much as possible, the user should not see the error.

[drewwells]

I mean, you don't need to show rec.file().unwrap_or("unknown") and rec.line().unwrap_or(0) in the log. Step to show error :

The error messages are not unique across the codebase, so I did not know which line was throwing the error. It's pretty standard to show the line number. The dev logger I use gives me a full stack trace when I log an error.

• db directory exist

  • only readable or writable -> show error to user with full db path
  • continue;

• db directory not exist

  • mkdir

    • failure -> show error to user with full db path
    • success -> continue;

As much as possible, the user should not see the error.

You want to attempt to create the directory before emitting an error? That shouldn't be hard to add.
I can attempt to make the directory even if it exists, ignore already existing errors. The rest of the code calling sqlite will then be the same but at least we attempted to create a missing directory first.

[SudoDios]

You want to attempt to create the directory before emitting an error? That shouldn't be hard to add.

Yes, it's easy.
I wanted you to complete it.
An error should be when nothing more can be done.

[drewwells]

DEMO
pointed sqlite to a non-existent directory. It created it and continued on

[2024-11-28 20:19:48.809 INFO src/config/mod.rs:128] Configs initialized file : configs.toml
[2024-11-28 20:19:48.809 INFO src/database/mod.rs:49] Database Sqlite initialized successfully

[SudoDios]

I have an idea bro.
Undo the edits you made in the /config/mod.rs file (because they are not needed).
and check in sqlite.rs that the directories are not read-only and writable (if they are, the user will be explicitly told in the error).
Okay ?

[drewwells]

I have an idea bro. Undo the edits you made in the /config/mod.rs file (because they are not needed). and check in sqlite.rs that the directories are not read-only and writable (if they are, the user will be explicitly told in the error). Okay ?

It's rustfmt and two lines to add filename:linenumber to log messages. Both those are useful, I can figure out how to revert rustfmt if you don't want the formatting changes.

[drewwells]

Updated...

[SudoDios]

@drewwells
This good ☺️.
Should I merge this ?

[drewwells]

@SudoDios good to merge!