Battle plan for dropping `HttpServer`

Developer
21

Do you mean browser refresh?
Then yes, the idea is that the current state (progress, backup names, etc) is correct when fetched from the server. If an update is missing in the client, it may show incorrect information. To counter this issue, a refresh will force-load everything in the state. It is a bit heavy handed, but not expected to happen unless there is a shaky connection.

This message is shown during startup of the server. In case the user forgot the password or has an autogenerated password, they will not be able to log in. The link shown is the same kind of link that the tray icon uses, which has a signin token with 5 min expiration. Using the link will log in without knowing the password, if done within 5 minutes of starting the server.

I think we can adjust this to only show if the password is auto-generated.

22

I was not able to reproduce it - sadly. In my case even if I restarted Duplicati entirelly, my browser was just reconnecting after 30s or something. I broke token deliberately and was able to reproduce 401 during WebSocket connection.
One thing is important that browser API for websocket is deliberately limited by standard for the security reasons. So instead of returning 401 and closing HTTP connection, we accepting WebSocket and returning custom code 4401 which is in the range of custom application codes.
https://www.rfc-editor.org/rfc/rfc6455#section-7.4.2

Those codes are visible in JS so we can change FE logic based on that - it shows FE modal window that says that you need to refresh page to continue.

Does it sounds fine for you?

It is in my own branch: GitHub - npodbielski/duplicati at feature/remove-httpserver

23

I will test again on my machine then, and see if I can write repro steps.

Yes, that sounds fine. I assume this is the normal way of handling it with WS.

24

@npodbielski I reworked it a bit again:

The idea is to not use the /api/v1/serverstate request to fetch the status.
Instead I changed the websocket to automatically send the latests state on connect.

If there is a 401 (or 4401) this will clear the access token and attempt to get a new one.
Other than that, I removed the open event listener, because the logic is handled by the message now appearing on connect.

I have tried various setups with expiring tokens, server restarts, etc.

If this PR works for you, can you merge it in your repo, and then I will merge that back into my branch, before merging to master.

25

If you do not want this endpoint, yeah totally it makes more sense to use just websocket.

Sure I will go through it. No problem. Thanks.

26

It was more that it was doing the same as the websocket, and the logic became harder to follow and debug with multiple paths.

I think we can also use websockets for the tray icon now and reduce overhead a bit there as well.

27

yeah sure.

I cleaned up the code a bit.

  1. There is no circular dependency between IStatusService and WebsocketAccessor so there is no need to use IServiceProvider.
  2. Changed WebsocketAccessor to use thread safe collection instead of locking. I think the only downside is that from some time to time client may be twice in a collection and receive some data twice but since this is mainly locally hosted website this will never happen. And order of those connection does not matter so it is better to use thread safe collection instead of locking.
  3. cleaned up a bit ServerStatus.js
  4. Just stumbled once onto strange behavior when page reloaded after reconnected via Websocket (I just restarted debug session) - pretty weird, I do not think it should do that since websocket server state update should have all the necessary data to refresh FE, right?
  5. Removed /serverstate endpoint and left just resume and pause. If you removed it on FE there is no need to leave it on BE.

I think it is good but I think I will play with it a bit more, maybe with more than one client just to make sure everything is good, but I think it should be fine.

28

It is currently used by the TrayIcon, so we need to either keep it in, or update the TrayIcon’s connection code to use WS.

Yes, the initial request should send all information from serverstate and call the broadcast methods. This should trigger a few extra requests, but generally, from the initial message from the BE, the FE should refresh fully.

For future updates, I think it makes sense to send things like notifications, progress state, task state, etc. via WS messages as well.

I will try your branch a bit more then.

29

Ok, I brought it back.

So full browser refresh? This is how you intended to do it? Right now it just reconnects to websocket for new updates. At least this is how I implemented it.

I would advise against it. I.e. imagine filling the backup information with secrets addresses and all of that, and booom! It reconnects and you lost it. That would be very annoying.
From what I saw there is like 20 properties that caries server information. If something is missing we should add it to this message to ensure that everything is refreshed on FE side.

Or we can have some small toast in the bottom saying that updates were interrupted: ‘please click hear to refresh the page’ but we should not do it forcefully.

Yes of course.

I found the code that was refreshing the page:

obraz

Part of the old code for handling /serverstate endpoint.
This is not part of my implementation so I think we can merge websocket fix to master - though I still think we should not do that if not totally necessary.

Otherwise I did not found any issues. Synchronization of data between many clients works.

30

I see this was unclear from my side. I meant that once the WS is connected, the BE will send the current state and the FE should be fully up-to-date after receiving the initial message. It will immediately request a bunch of other information, such as the list of backups, the list of notifications, etc. and this could easily be packed into the WS request, such as having the FE sending the latest versions in an initial request, and the BE giving data that is missing.

Maybe that is a better solution. Since the BE does not store old messages, lost messages cannot be resent to the FE. So an outage that misses a few messages will be inconsistent, but maybe we can fix it as mentioned above.

Right now this will trigger a page reload with the code you found.

Great! I will try to merge it tomorrow then.

I am not too worried about the page refresh, it should only happen on really flaky connections and never on localhost. We can remove that code once the WS can provide all information, so we can auto-sync once the connection is re-created.

31

It seems I have discovered a race condition that happens now because the events are super fast.
The sequence is something like this for FE:

  • Start task
  • Wait for task to complete
  • Call completion task

This is done in ServerStatus.js in the method callWhenTaskCompletes().
It looks like the task can start and finish before the FE can register the callback, which makes the FE stuck waiting for the task to complete. I think it has always been possible to get in to this state, but it happens more often now due to changed timings. Especially with the websocket, it is possible to get updates much faster than before, making it even more pronounced.

My workaround is to not rely on the websocket for reading the task state, but instead poll for the task state, which seems more reliable. We should rewrite that part so the BE will send task started/completed events instead.

This requires some rewriting in the event area in the BE and some work to re-implement this in the FE, so I will postpone this to later, as the PR is quite heavy already.

32

I think simpler would be store timestamp of last message and just request update with that timestamp. If some part of updates were lost or some was not send: timestamp will be smaller than on server side.
I am not sure if there will be necessity to store such timestamp for each type of notifications (i.e. server state, current backup process etc.) or just one. Probably it will be less error prone with multiple timestamps.
Of course it will be working with whole data too. Just will be much more complex to traverse all messages and check if and what is missing. It could be time consuming if some part of the data model will be recalculated every time instead of cached somewhere.

Sure no problem. But I will have to investigate first since I am not sure what this is about.

33

There are programs such as duplicati-client that work with the HTTP API directly.

means they need to rewrite their authentication, but how much further does it go?

Potentially, could there be compatibility issues of mismatched Duplicati parts too?

For example, TrayIcon or browser code might be on different versions than server.

34

Good point. The point of the IDs is to see if an update is missed. Say the FE has ID=2 and the BE sends ID=4, then you know you have missed an update, and the UI state is unreliable. If were are only using a timestamp, we can just know if something has updated, not if any messages are missed.

But we can make everything simpler by just reloading the relevant state every time, instead of attempting to patch the current state with messages.

Yes, and more error prone too. I think for the amount of data processed by the WebUI there will be no problems choosing a simpler way.

Yes, that needs to be updated to support the new auth scheme. i suppose a good way would be to release a new canary so there is something to test with. If @Pectojin is interested, the example script has been updated to work with authentication, so it should be trivial to support that part.

The breaking points are:

  • Rewritten auth support
    Now everything uses standard bearer tokens, no special hashing logic, no xsrf tokens
  • The import API call has changed to not use the form-file, but instead just have input as JSON.
    This greatly simplifies everything, including the response.
  • There may be one or two places where the capitalization changed in the response.
    Not sure exactly why, as the underlying data format is the same
  • No more use of application/x-www-form-urlencoded
    Simplifies things greatly as only JSON input is accepted

For duplicati-client it does not matter, but since the introduction of a websocket, the endpoint for serverstate and progressstate may be removed in the future.

In my view, the expectation is that the TrayIcon follows the server version, so I do not see that as a problem.

The API has been stable, at least for the parts used by the TrayIcon for many years, but since we are updating to .NET8 anyway, I think now is a good time to make the breaking change.

35

Sorry I read it multiple times and still not sure if you are agreeing with me or politely disagreeing. :smiley:
So we are keeping IDs for now?

As I can see it:

For Ids:

pros:

  • simpler a bit
  • already implemented so no changes
  • handling everything in single message is simpler to code for now

cons:

  • resetting ids with server restart (another code logic to handle)
  • you can’t have multiple messages with single ID so costly on BE
  • needs FE full update (again costly with full refresh)
  • having one single message type will be a problem at some point (god classes and similar, hard to maintain in the long term)

Yes, but with websockets you also need to send messages from time to time to makes sure it is still connected and works. How often? It depends on implementation but usually 5-10 min and connection is dropped.
Again probably not an issue for localhosting Duplicati, but one message every other minute would be good. Server state is sent more often than a minute. So I do not think it will be a problem. Still it is not like with WS you can just forget about sending any data for hours.

Come to think about this, major issue here is that server sends incremental updates not a whole state of an application and expects clients to take care of them themselves. But and the same time does not sends any data in a manner that allows the client to tell if state is correct at all time.

Yes there is an ID but server may be idle for a while and ID is incremented every time some data is sent. Does missing an update does definitely mean that UI is incorrect? No.

I think we need something that will be sufficient for following cases:

  • client have no information and request full update
  • client have latest information and we are sending just keep-alive kind of messages (so almost no data)
  • client missed a few updates but there were no changes in the meantime (so no need to update)
  • client missed few updates but state changed and differs from the real one (need to update) would be nice to be able to say what is missing and what need to updated (so no full refresh)

For now, for me, it looks like that we need to split message into multiple messages, connected to some areas: i.e.

  • server state
  • list of backups
  • notifications
  • etc

Each of those states probably should be having a checksum of some kind (i.e. Md5) if any of the data changes server sends an update and checksum on its side. If client after calculating checksum on its side says their matching - nothing is happening. If they differs client requests update for specific area.

So probably timestamps are not best idea after all.

36

I meant to agree :smiley:

Yes, that is what the ID is meant to handle, but it puts a lot of work into the FE.

I think your suggestion with using a timestamp is simpler (for the FE) but it requires changing the BE to actually send full states.

Given the small amount of data, I do not see this as a problem, but a simplification that removes the potential for errors.

I am positive for the timestamp idea. It changes the way we send data though.

For the initial connection, the BE needs to send the items you list above.

We can use a WS message that is something like “send me everything changed since ‘last-timestamp’” and you get everything, but things that are not changed are just null. Or simpler yet, they are filled anyway.

The response message from the WS is then the same as the update message, something like:

{
  "last-timestamp": 1234,
  "backups": [ ... ],
  "notifications": [ ... ],
  "serverState": { ... },
  "progressState": { ... },
  ...
}

This simplifies the FE quite a bit, but it requires a re-write in both the BE and FE to handle a single message and dispatch it.

37

While it’s true that the TrayIcon talking to its own hosted server should match, there’s also its:

--no-hosted-server
Set this option to not spawn a local service, use if the TrayIcon should connect to a running service.

Also noting that the removal of autoupdater removes one possible way for versions to diverge, however .zip file installs can do it, and remain available for users to install wherever they like.

Browser JavaScript wasn’t commented on at all. If I keep a tab open, and switch the Duplicati version back and forth, will all the browser code be downloaded again, or is it done manually?

I know we’ve made progress on purging wrong-versioned JavaScript, but I’m not sure it’s total.

38

There is no check for version compatibility in the current TrayIcon. It will simply fail to connect if the server is updated, as it does the login process wrong. I think this has always been the case, but it has not been a problem before because the API was the same.

I would make sense to update the TrayIcon to check if the API is compatible, but this will of course not work back in time to existing installations.

This is not directly supported.

If you switch versions, it will fail to connect and show the reconnect box. Reloading the page will fix it all.

The updated server handles the index.html to not be cached, so it will reload every time (v2.0.8.1 does not correctly handle this, the browser may serve a stale version).

Each resource is loaded with the version number in the URL and cached, so reloading the page will not require the html/js/css files to be reloaded.

This is not the correct fix though, the real fix is to use a “compiler” that packages the resources into bundles for the browser. Each bundle has a unique hash value, so caching is not an issue. Updating to a recent Angular will fix these issues and many others.