Skip to content

Add --rx-bytes flag to enable logging of received bytes in hexadecimal#81

Open
jpdillingham wants to merge 3 commits intosoulfind-dev:masterfrom
jpdillingham:rx-bytes
Open

Add --rx-bytes flag to enable logging of received bytes in hexadecimal#81
jpdillingham wants to merge 3 commits intosoulfind-dev:masterfrom
jpdillingham:rx-bytes

Conversation

@jpdillingham
Copy link
Copy Markdown
Member

I wanted to definitively prove that slskd/Soulseek.NET is sending the expected bytes for a message and this was the best way I can think of to do it. To avoid opting everyone wanting --debug in to the logging of raw message bytes, I added the --rx-bytes that works in conjunction with --debug (and does nothing but log a warning unless both are supplied).

Example output:

$ ./bin/soulfind --debug --rx-bytes
[DB] Using database: /home/jp/src/soulfind-fork/soulfind.db
[DB] Checking database integrity
[DB] Database integrity verified
[DB] Updated config value port to 2242
[DB] Updated config value max_users to 100000
[DB] Updated config value private_mode to 0
[DB] Updated config value motd to Soulfind %sversion%
♥ Soulfind Mar-25-2026 process 547657 listening on port 2242
[Conn] Connection attempt accepted
[Conn] Registered connection socket
[Msg] Receive <- ULogin (code 1) <- from user [ unknown ] (01 00 00 00 09 00 00 00 70 ... <truncated>)

I used AI to help with the writeln syntax in messages.d.

@mathiascode
Copy link
Copy Markdown
Member

I'm wondering if it would be better to do something like --debug=simple and --debug=verbose, and default a bare --debug to simple? Having a separate flag that depends on another feels a bit awkward.

@slook
Copy link
Copy Markdown
Member

slook commented Mar 27, 2026

Better to imply the required flag rather than the error if it's missing.

It is desirable to have fine-grained control of the output, otherwise sifting through all of the data to find the bit you're looking for will become too overwhelming when in reality you are only going to be testing one part of the protocol at a time (perhaps only one specific message code or set/range of codes) during any given session.

Simpler separate flags for each that may category desired...

-a --all --verbose "Enable all logging categories (--verbose implies --all --rx --tx)"
-d --db "Enable debug logging of database operations"
-c --conn "Enable logging of user connections"
-m --msg [code(s)] "Enable logging of protocol message(s)"
-r --rx "Enable logging of received bytes (implies --msg)"
-t --tx "Enable logging of transmitted bytes (implies --msg)"

Suggest dropping the "debug" terminology altogether because its ambiguous if that's for debugging Soulfind itself, or if it's really intended for the purpose of debugging the developer's client that they're working on?

@mathiascode
Copy link
Copy Markdown
Member

Some more control would be good, but I have some concerns about adding generic separate flags like this. E.g. -d and --db would clash with the existing --database flag for specifying the db path.

Maybe a --log flag that can take one or several similar values like you proposed? That would leave some room to add new values in the future without blowing up the number of flags.

@slook
Copy link
Copy Markdown
Member

slook commented Mar 27, 2026

Yes that would be much more sensible.

I'm not saying all these have to be implemented, just exploring what kind of permutations this syntax could potentially allow...

-l or --log "Enable all logging categories"
--log "user" "Enable all logging categories for user (quoted)"
--log conn "user" "Enable logging of user connections"
--log db "Enable logging of database operations"
--log msg "Enable logging of all protocol messages"
--log msg "user" "Enable logging of all protocol messages for user"
--log r "Enable logging of received protocol messages"
--log t "Enable logging of transmitted protocol messages"
--log rx "Enable logging of received bytes"
--log tx "Enable logging of transmitted bytes"
--log x "Enable logging of all bytes (equivalent to msg rx tx)"
--log 1 "Enable logging of protocol message code" (e.g. 1 = Login)
--log 1 r or --log r 1 or --log msg 1 r Enable logging of received code 1 message
--log 1 t or --log t 1 or --log msg 1 t Enable logging of transmitted code 1 message
--log 1 rx or --log rx 1 or --log msg 1 rx Enable logging of received bytes for message code 1
--log 1 tx or --log 1 tx --log msg 1 tx Enable logging of transmitted bytes for message code 1
--log 1 x or --log x 1 Enable logging of bytes for message code 1
--log 1 42 123 Enable logging of code 1 and 42 and 123 messages
--log 1 42 123 rx Enable logging of received bytes for code 1 and 42 and 123 messages
--log r "user" 1 Enable logging of received code 1 message for user
etc.

For most of these it wouldn't matter if msg was specified or not, so that can be optionally omitted unless any of them might also apply to conn and db, but any integer, rx or tx param can only be assumed to apply for msg.

For the purposes of this PR would be accepting either --log rx or --log msg rx (rx implies msg).

@mathiascode
Copy link
Copy Markdown
Member

Better to start with some basic categories first. I could take a look at implementing that and see what's needed in our arg parser.

@mathiascode
Copy link
Copy Markdown
Member

Initial implementation of --log argument in #82.

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.

3 participants