Add Reboot Delay

[geelongmicrosoldering]

Hey there,

This probably isn't the best way to do this, but I noticed the need for a response isn't fulfilled in PreferencesPage.jsx#L92

We use traccar with Cloudflare, so the result for us is seeing the source of the CF 502 page as an error, because the endpoint immediately becomes unavailable and never returns an ok response. Presumably if you don't use CF the promise just doesn't resolve because the server can't return anything, so most wouldn't see an error if the reboot is successful.

The reboot works fine either way. This just provides a response and schedules the reboot to occur 3 seconds later. The response has no body. I suppose later you could return a message like "Rebooting in 3 seconds" and display it to the web user. This is just one way to handle the expectation of an ok response, by providing one and rebooting afterwards.

I would have offered additional feedback but I actually have no idea how to do a PR that includes traccar-web

Just an idea anyway

Cheers

[tananaev]

What's the problem with CF 502 page as an error?

[geelongmicrosoldering]

Basically you get an error upon a successful action, as well as an unsuccessful action. So if the server reboot is successful you get this (apache and nginx users probably have something similar):

The error also then persists because the X extends off the viewable area, so you have to force refresh to get rid of it. Basically the browser is expecting a response that can never come.

If you were to display an error on a successful action, it probably makes more sense for the error to show that the action was successful (im not sure throwing an error is the best way to show success, but here it is 😄 ):

  const handleReboot = useCatch(async () => {
    const response = await fetch('/api/server/reboot', { method: 'POST' });
    if (!response.ok) {
      throw Error(await response.text());
    } else {
      await response.text();
      throw Error("Reboot Successful. Please wait...");
    }
  });

of course you'd probably want useTranslation instead of a fixed string. that was just an example.

Doesn't effect function, just tells the user they they have indeed rebooted, and didn't run into an error

[tananaev]

I suggest we just ignore response and maybe reload the page.

[geelongmicrosoldering]

I suggest we just ignore response and maybe reload the page.

page already reloads automatically when the server comes back so that's no issue.

ignoring response works but also means if a users admin privilege's are revoked while they are on pref page or for some other reason the reboot is unsuccessful (database issue, maybe the reason they are trying to reboot?), you don't get the error explaining it.

Both of those options get rid of the full page error though. I can close this pr and submit one on traccar-web if you want to go that way.

We could also enumerate the response code and throw either an error, or consider 502/503/504 a success, which should basically result in the user getting appropriate feedback, but with a single pr on traccar-web. For example:

  const handleReboot = useCatch(async () => {
    const response = await fetch('/api/server/reboot', { method: 'POST' });
    if (response.status != '502' && response.status != '503' && response.status != '504') {
      throw Error(await response.text());
    } else {
      throw Error(t("sharedLoading"));
    }
  });

That gets rid of the extra thread, the delay, and has the same outcome as the current release, with better user feedback. It seems like the best option. It also honors the language translations

What do you think?

[tananaev]

What if we just do something like this:

  const handleReboot = useCatch(async () => {
    const response = await fetch('/api/server/reboot', { method: 'POST' });
    throw Error(response.statusText);
  });

[geelongmicrosoldering]

What if we just do something like this:

  const handleReboot = useCatch(async () => {
    const response = await fetch('/api/server/reboot', { method: 'POST' });
    throw Error(response.statusText);
  });

it works but you miss out on actual errors like this (granted, you have to revoke someones admin permission while they are on the pref page).

I imagine there's edge cases where the error might be useful, like database corruption that they think is traccar related so they keep rebooting and creating issues asking for help. having the database error show up might prompt them to do a google search first

You also get a response that suggests there was a problem with your request, when your request was successful (the server did reboot and all is well)

But its definitely better than a full page error lol

[tananaev]

Reboot usage should be very rare. It's not a normal API. So I think it's ok to not be very user friendly in terms of messaging. I just want to make sure we don't overcomplicate something simple like this.

[geelongmicrosoldering]

Reboot usage should be very rare. It's not a normal API. So I think it's ok to not be very user friendly in terms of messaging. I just want to make sure we don't overcomplicate something simple like this.

yeah, that's fair. well your proposed solution definitely works then. did you want me to submit it in a pr on traccar-web or did you want to just edit it yourself?

we can close this pr when you are ready

[tananaev]

If you have time to do it, yes, please submit the PR.