hoytech / strfry

a nostr relay
GNU General Public License v3.0
489 stars 96 forks source link

Considerations for logging and exit codes for `styfry sync` #124

Open braydonf opened 1 week ago

braydonf commented 1 week ago

I'm running a program that fetches all my contacts/follows (kind 3, NIP-02) and also their relay list metadata (kind 10002, NIP-65) and then execs this command for each contact and relay:

/usr/local/bin/strfry --config=/etc/strfry/strfry.conf sync wss://example.tld --filter={\"authors\":[\"<hex-pubkey>\"]}

I've noticed a few things:

Exit codes

There are several relays that do not yet support negentropy syncing and have received NOTICE messages from relays, a few for example:

The process will stay open, assuming indefinitely, so it's necessary to put a timeout on the process and exit it. This works so long as there is some output indicating progress. I've only noticed the timeout clobber a working process once (so I'll likely need to increase my timeout).

However, I am wondering:

  1. If there should be a timeout within strfry sync as it has a better idea if there is activity.
  2. If the should be a protocol addition for relays to send an equivalent to CLOSED if a protocol extension hasn't been implemented. The NOTICE message is intended to be human readable and attempting to machine parse it seems futile.

Stderr and stdout

All info logs seem to be going to stderr instead of stdout, is this intentional?

Logging large events

Some of the reject events are logged including the ones that have failed with "reason: event too large". Is it a good idea to log these events, it seems like this could be a DoS vector to fill disks.

hoytech commented 1 week ago

Thanks for this! You're probably right and we can make some heuristics for detecting whether a sync failed. That's a great list, I appreciate you compiling and posting it.

Yes, log messages go to stderr by default with loguru. Several strfry commands use stdout (import etc) so logging there would interfere with it.

Good point on the logging full events. This may not make sense by default: I will look into this.