REST endpoint Connections total incorrect paused status

jerryjacobs · January 8, 2022, 2:55pm

Syncthing macOS uses the pause and resume endpoints to manage the connections in one go. But for some reason all connections are paused but the total.paused returns false. This is not what I expect. See webgui chrome JSON trace.

calmh · January 8, 2022, 3:34pm

paused there is false for the same reason that address and crypto are the empty string, etc. They are not fields that are filled out for the total, yet they are present in the type used in the reply. Arguably that’s a bug and a different type should be used.

github.com

syncthing/syncthing/blob/bc3d306dd73037a85c4180181d9c451a881119b4/lib/model/model.go#L772-L778

    
      
          	res["total"] = ConnectionInfo{
          		Statistics: protocol.Statistics{
          			At:            time.Now().Truncate(time.Second),
          			InBytesTotal:  in,
          			OutBytesTotal: out,
          		},
          	}

jerryjacobs · January 8, 2022, 3:47pm

This snippet makes more sense now how it works. I was misled by the name ‘total’. But is only to have total In and Out bytes, it would not be hard to calculate the total paused state for this total connection. Which in my opinion adds extra functionality to this entry. Currently the total paused state is calculated in-application.

acolomb · January 9, 2022, 11:41am

So what would you expect as the “total paused state”? True if any remote device is paused or if all remote devices are paused?

Same goes for connected, I’d assume we say connected as soon as one connection is active. Or should we just remove all these unpopulated members?

I’ve opened API: /rest/system/connections has misleading "total" entries · Issue #8103 · syncthing/syncthing · GitHub to track this, but let’s keep the discussion about it here for now.

jerryjacobs · January 9, 2022, 2:56pm

I would prefer total.paused = true if all remote devices are paused. For total.connected = true the same story (all devices). And if we agree on this we should document its behaviour. It’s also a matter of taste which thing we decide. Having a total connection in the connection list is still a special purpose entry not exposed on a different endpoint.

calmh · January 9, 2022, 3:06pm

The main point of the total entry is to give the total traffic counters, which doesn’t reset when connections come and go. So this is functionality that can’t be recreated by just summarizing the other connection entries. Same can’t be said for paused and such.

acolomb · January 9, 2022, 5:48pm

I agree, the extra info is easily reproducible and the semantics would be ambiguous. Let’s just remove the fields.

jerryjacobs · January 10, 2022, 6:54am

I totally agree on you both, we should indeed remove those fields for the total entry and document it slightly a little better what the purpose of the total entry is.

For the application we will derive the all/any paused state by ourselves which is no problem at all.

Thanks for the good discussion.

system · February 9, 2022, 6:55am

This topic was automatically closed 30 days after the last reply. New replies are no longer allowed.