Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] :electron: hosting the sync-server with the desktop app (POC) #3631

Draft
wants to merge 71 commits into
base: master
Choose a base branch
from

Conversation

MikesGlitch
Copy link
Contributor

@MikesGlitch MikesGlitch commented Oct 10, 2024

The goal

Free server hosting via the desktop app. We'd run the sync server inside of the desktop app, expose it to the internet via ngrok (or similar) and provide a QR code/url for the user to connect their phone to the server.

We could provide an option to start actual-server when the computer starts up. If we did this the server would be alive whenever the computer is on. If the user downloads the pwa to their phone they could budget even when the server is offline - it would sync when the computer is turned back on.

Todos:

  • Get the sync server working in the app
  • Expose the sync server using ngrok
  • Test this as my daily driver
  • Package the app and make it fully functional
  • Prevent error message "We had problems syncing your changes. Please report this as a bug by [opening a Github issue]" when the server is offline. I think it's because it hits ngrok first - ngrok can't connect to tunnel.
  • Fix server status appearing as offline when it finishes starting after web client loads [Bug]: says "server offline" when it actually came back #3128
  • Automatically set the server URL when the user uses the Electron Server. Update the UI to say that we're connected to "Local Server" or something instead of the server URL.
  • Socket hangup issue - when closing and opening the budget fresh when connected to ngrok server url.
    • I switched to udinci and it was still happening. It seems related to NGROK - I don't get it when I connect directly to the server.
    • When I connect via the web I don't get it at all.
    • This error can be avoided by connecting direct to localhost via electron and it doesn't occur on web/pwa. It's avoiding the error but not fixing it - but that may be satisfactory because it's better to connect to the server via localhost on electron for better offline functionality
  • Make progress towards getting the sync server in the main Actual repo
    • This will be done properly in a seperate PR - this is just a test branch
  • Change the server port to something else so it doesn't clash with local dev testing (not 5006)
  • Allow the server to run on https within localhost - this will require setting up a certificate - this can be optional - setup a server config ui?
    • Make the electron app manage the SSL so that users don't have to set it up
  • Build the UI
    • on server setup page when electron, push people towards using this functionality - still allow external server functionality
    • a settings area/modal to allow server configuration for initial setup and edits - Should probably live on file management page and under the server status dropdown.
    • a start and stop functionality for the server under the server status dropdown
  • Tidy the code
  • Test on
    • Window
    • Linux
    • Mac

In a follow up feature:

  • Provide an option to start actual-server when the computer starts up

Known issues

  • server /info endpoint isn't returning data - it means we can't see what version the server is on. I think it's to do with it not finding the package.json file. We're using an external package for it.

Next: Figure out the best way to bring actual-server into the actual repo

Expose the running server to the internet

Manged to get it working with ngrok. Ngrok's big and they give you a free domain. It works a treat. I haven't tested it in anger, I'm sure it will disconnect me after an hour or so so we may need to add logic to reconnect.

Thoughts on bundling the actual-server code

  • If actual-server was in the actual repo we could bundle using electron-builder. The package size seemed good when I tried that. This seems like the right way forward for all kinds of reasons - some not related to electron - type sharing, debugging, simlicity etc.

Old thoughts:

  • Actual-server bundle is huge - consider using webpack or https://github.com/vercel/ncc . Webpack seems promising, they have targets for electron and node.
  • BetterSqlite3 is complaining that it was compiled for server on a different version of node than electron. We'll also run into the arch issue so we'll need to fully make that external in the actual-server bundle and use the one we have in electron.
  • If we bundle, the bundle should only be used for electron (if we want to bundle for normal use it will be a separate bundle)

Server sync notes

  • We could warn the client against syncing if the client version is greater than the server version.
    • This means we should never have out of sync migrations
    • We'd always know the server version & client version at this point - we would have already migrated but we wont sync it with the server unless it is safe to do so.
    • Downside - if it takes 2 weeks to update the server the user is unable to sync until it is updated

Copy link

netlify bot commented Oct 10, 2024

Deploy Preview for actualbudget ready!

Name Link
🔨 Latest commit 606fe7e
🔍 Latest deploy log https://app.netlify.com/sites/actualbudget/deploys/67535d4a757645000857bc68
😎 Deploy Preview https://deploy-preview-3631.demo.actualbudget.org
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

github-actions bot commented Oct 10, 2024

Bundle Stats — desktop-client

Hey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle.

As this PR is updated, I'll keep you updated on how the bundle size is impacted.

Total

Files count Total bundle size % Changed
9 5.34 MB → 5.34 MB (+470 B) +0.01%
Changeset
File Δ Size
src/components/manager/BudgetList.tsx 📈 +399 B (+3.44%) 11.32 kB → 11.71 kB
src/browser-preload.browser.js 📈 +71 B (+1.75%) 3.96 kB → 4.03 kB
View detailed bundle breakdown

Added

No assets were added

Removed

No assets were removed

Bigger

Asset File Size % Changed
static/js/index.js 3.34 MB → 3.34 MB (+470 B) +0.01%

Smaller

No assets were smaller

Unchanged

Asset File Size % Changed
static/js/usePreviewTransactions.js 1.64 kB 0%
static/js/resize-observer.js 18.37 kB 0%
static/js/indexeddb-main-thread-worker-e59fee74.js 13.5 kB 0%
static/js/AppliedFilters.js 20.96 kB 0%
static/js/narrow.js 82.55 kB 0%
static/js/BackgroundImage.js 122.29 kB 0%
static/js/wide.js 242.95 kB 0%
static/js/ReportRouter.js 1.51 MB 0%

Copy link
Contributor

github-actions bot commented Oct 10, 2024

Bundle Stats — loot-core

Hey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle.

As this PR is updated, I'll keep you updated on how the bundle size is impacted.

Total

Files count Total bundle size % Changed
1 1.26 MB 0%

Changeset

No files were changed

View detailed bundle breakdown

Added

No assets were added

Removed

No assets were removed

Bigger

No assets were bigger

Smaller

No assets were smaller

Unchanged

Asset File Size % Changed
kcab.worker.js 1.26 MB 0%

@MikesGlitch MikesGlitch changed the title [WIP] :electron: server hosting [WIP] :electron: Experimental: hosting the sync-server locally Oct 10, 2024
@MikesGlitch MikesGlitch changed the title [WIP] :electron: Experimental: hosting the sync-server locally [WIP] :electron: hosting the sync-server locally Oct 10, 2024
@MikesGlitch MikesGlitch changed the title [WIP] :electron: hosting the sync-server locally [WIP] :electron: hosting the sync-server with the desktop app Oct 10, 2024
@Jonathan-Fang
Copy link
Contributor

This seems very cool!

@MikesGlitch MikesGlitch changed the title [WIP] :electron: hosting the sync-server with the desktop app [WIP] :electron: POC: hosting the sync-server with the desktop app Oct 12, 2024
@MikesGlitch MikesGlitch changed the title [WIP] :electron: POC: hosting the sync-server with the desktop app [WIP] :electron: hosting the sync-server with the desktop app (POC) Oct 12, 2024
@@ -37,6 +37,7 @@ fi
yarn workspace loot-core build:node

yarn workspace @actual-app/web build --mode=desktop
yarn workspace @actual-app/crdt build
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

crdt is a dependency of actual-server. Build it before packaging the sync-server

}

if (Array.isArray(accountId) != Array.isArray(startDate)) {
console.log(accountId, startDate);

Check failure

Code scanning / CodeQL

Use of externally-controlled format string High

Format string depends on a
user-provided value
.
);
}
if (Array.isArray(accountId) && accountId.length !== startDate.length) {
console.log(accountId, startDate);

Check failure

Code scanning / CodeQL

Use of externally-controlled format string High

Format string depends on a
user-provided value
.
}

try {
await fs.writeFile(getPathForUserFile(fileId), req.body);

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.
packages/sync-server/src/app-sync.js Fixed Show fixed Hide fixed
}

res.setHeader('Content-Disposition', `attachment;filename=${fileId}`);
res.sendFile(getPathForUserFile(fileId));

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.
// Source: https://expressjs.com/en/guide/error-handling.html
return next(err);
}
console.log(`Error on endpoint ${req.url}`, err.message, err.stack);

Check failure

Code scanning / CodeQL

Use of externally-controlled format string High

Format string depends on a
user-provided value
.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants