Restart server silently when adding self signed cert and add some logs
#3431
Conversation
✅ Deploy Preview for actualbudget ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Bundle Stats — desktop-clientHey 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
Changeset
View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger
Smaller No assets were smaller Unchanged
|
Bundle Stats — loot-coreHey 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
Changeset No files were changed View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger No assets were bigger Smaller No assets were smaller Unchanged
|
| }, | ||
| }); | ||
| } catch (error) { | ||
| console.error(error); // log error |
There was a problem hiding this comment.
Adds a bit extra logging - this will help with cert/network/other errors
MikesGlitch
force-pushed
the
electron/restart-server-silently-and-logs
branch
from
20be64a to
48e56f4
Compare
packages/desktop-client/src/components/manager/ConfigServer.tsx
Outdated
Show resolved
Hide resolved
| } | ||
| } | ||
|
|
||
| const isMounted = useRef(false); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
💭 thought: I think this doesn't actually work as intended.
What I think this should do: whenever self signed cert (serverSelfSignedCert) changes - the electron server is restarted.
What actually happens:
- electron server is restarted the first time
serverSelfSignedCertchanges - if the cert changes again (or is removed) - the server does not restart anymore thus is broken again
There was a problem hiding this comment.
If you move this to a useCallback that is called when setServerSelfSignedCert is called - that will make it work better IMO.
There was a problem hiding this comment.
What actually happens:
electron server is restarted the first time serverSelfSignedCert changes
if the cert changes again (or is removed) - the server does not restart anymore thus is broken again
That's not what I'm seeing:
If you move this to a useCallback that is called when setServerSelfSignedCert is called - that will make it work better IMO.
Yeah that was my original plan but setServerSelfSignedCert dispatches an action that writes to the file system, so there's a delay between dispatching and having an updated value for serverSelfSignedCert.
If the delay is too great, we restart the server before changing the value - that's why I changed it to watch for the value change rather than restating there.
There was a problem hiding this comment.
If the delay is too great, we restart the server before changing the value - that's why I changed it to watch for the value change rather than restating there.
Hmm.. so it all comes down to how long the delay is. And if the delay is sufficiently long - even the useEffect solution wouldn't work. Could we await for the result of this dispatched action? Or perhaps listen to a callback of some sort?
There was a problem hiding this comment.
I've passed a callback on the dispatch which will trigger the restart. This feels cleaner than what I had 👍 3600b0c
…ogs (actualbudget#3431) * restart server silently on add self signed cert and add some logging * release notes * fix name * updating names to be more specific * removing setloading * feedback
* marked files for translation * added release note * fixed linting warnings * 🐛 fix account filters being overridden (#3441) *
Selecting a self signed certificate no longer restarts the app. Instead we restart the server process silently. The user shouldn't notice.
I've also added some logging for errors given by nodeFetch. That should help people diagnose cert/network related erros.