I did convince myself and changed the config option to be error-on-failed-packet-sends. So when true it throws an error, and false logs a warning.

Why not have a option to disable the communication attempt directly instead of ignoring the error?

updateRequirements can't do anything on Velocity if it can't communicate with the backend, so if it is expected to fail, I don't think there is any reason to try calling it. If you do assume it will always work, I think there should at least be a warning when a method does not do what it is supposed to. When updateRequirements fails, it can cause confusing desyncs where the client tells a player they can access a command when they really can't.

I didn't think correctly, making this as disable-warning is good, sorry.

No problem :) I wasn't exactly sure what you meant before

But why is it that the error is downgraded to a warning? I mean, if you set a config option to disable an exception I'd except it to be completely silenced. I understand that you still want a warning because something went wrong, but then why have a config option in the first place?

My thoughts were based on this test setup I made.

Set<Player> canUse = new HashSet<>();

new CommandAPICommand("toggle")
    .executesPlayer(info -> {
        Player player = info.sender();

        if (canUse.contains(player)) {
            canUse.remove(player);
        } else {
            canUse.add(player);
        }
        CommandAPI.updateRequirements(player);

        boolean useState = canUse.contains(player);
        player.sendPlainMessage("Can use: " + useState);
    })
    .register();

new CommandAPICommand("test")
    .withRequirement(source -> {
        if (!(source instanceof Player player)) return true;

        return canUse.contains(player);
    })
    .executes(info -> {
        info.sender().sendPlainMessage("Success!");
    })
    .register();

The difference between an exception and a warning is that the exception interrupts the control flow. In this example, if updateRequirements throws an exception, then the player#sendPlainMessage step gets bypassed. If updateRequirements just logs a warning, then the surrounding logic can continue as if it did work.

The warning message can also be ignored by turning on the silent-logs option, so I thought this provided more possibilities. You can have error, warning, or nothing. Although, yeah, that does maybe conflict with other messages since you'd be turning off all other warnings and info logs as well.

I guess another option which does not show the warning is the user checking in advance whether the method would fail:

if (player.getCurrentServer()
    .map(server -> CommandAPIVelocity.get().getMessenger()
        .getConnectionProtocolVersion(server) > 0
    )
    .orElse(false)
) {
    CommandAPI.updateRequirements(player);
} else {
    // `updateRequirements` would have failed
}

This does give the most control to the developer what to do. I concede it isn't very good though since it's not documented and very implementation dependent, so a developer wouldn't know to do this and it could change. Maybe this logic could be put into a willUpdateRequirementsWork helper method?

Do you have use case where a different solution would make more sense?

The warning message can also be ignored by turning on the silent-logs option, so I thought this provided more possibilities.

I did not think of that. Well then I have nothing to say. Apologies

👍 No problem! Thanks for clarifying.

I just realized that this needs a docs PR to update the default config to include the new value.

I changed the config option to be make-networking-exceptions-warnings with default false. When true, exceptions that happen while sending or receiving packets will be caught and logged as warnings instead (which can be fully hidden by setting silent-logs to true).

I also added enable-networking, which is false by default. Networking is currently only useful to make updateRequirements work on Velocity, so server owners usually don't need this functionality.

I think this PR is good for review. Once it is merged, I will be certain about the names of the config options and will make a PR to add them to the docs.