mirror of
https://github.com/1Remote/1Remote.git
synced 2026-04-25 13:36:03 +03:00
[GH-ISSUE #395] Intrusive MySql Server connection error #339
Labels
No labels
area-configuration
area-ct-app
area-ct-rdp
area-ct-remoteapp
area-ct-ssh
area-ct-vnc
area-launcher
area-list
area-tags
area-teamwork
bug
chore
dependencies
general-build/ci
general-performance
general-refactor
general-security
general-supportive
general-ux
meta-documentation
meta-enhancement
meta-enhancement
meta-feature
meta-help-wanted
meta-unknown-error
priority-hi
priority-low
pull-request
question
resolution-duplicate
resolution-invalid
resolution-wontfix
stale
task-put-off
task-still-considering
task-working-in-progress
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
starred/1Remote#339
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @majkinetor on GitHub (Apr 26, 2023).
Original GitHub issue: https://github.com/1Remote/1Remote/issues/395
Originally assigned to: @VShawn on GitHub.
Describe the bug
When remote database becomes unavailable for any reason, there is this error:
Expected behavior
There should be no intrusion, especially since the error can resolve on its own. It seems that 1RM needs some form of internal Log viewer wiht marker for ciritical errors, warnings, currently erroring etc.
Additional context
This happens when VPN goes down to my company. Also when Windows is locked, after some time, I am always greeted with this message. VPN log is unremarkable but that is besides the point, the error is intrusive and constant.
@VShawn commented on GitHub (Apr 27, 2023):
I only checked the MySQL connection once during startup, but it should be changed to check the connection before any data operation. Additionally, I was lazy and applied the SQLite connection program to MySQL, which now I realize is problematic. I need to redesign this part.
@majkinetor commented on GitHub (Apr 27, 2023):
Wait what ?
This message happens while 1RM is already running in the background. When do you check for remote changes? On activation?
BTW, I think this should be redesigned to support "storing backend" so that new backends could be easily added later without messing with 1RM internals: PostgreSQL, JSON file, git repository (git mmmm :) ) or whatever. It makes sense that this should be apstracted via some nice Interface.
@VShawn commented on GitHub (Apr 27, 2023):
I check the
connected flagduring startup and when database profile was changed.while 1RM is already running in the backgroundand VPN shutdown, 1RM do not realize it lost connect since theconnected flagnot changed. the 1RM can still run the SELECT / INSERT / UPDATE / DELETE from db. When executing the SQL statement, I provide a final security check. If the statement fails to execute, a prompt will pop up. That why you see the pop upwhile 1RM is already running in the background.I provide a periodic check.
Emm what do you mean the "storing backend"? I've never heard this word before. do you mean something like Amazon S3 / Alibaba Cloud OSS? Ann...nd git is definitely not a good storage backend :)
a good news is is that I have designed the datasource interface in 1RM and have already implemented it on SQLite and MySQL. Means as long as the interface is implemented based on PostgreSQL, PostgreSQL can be integrated into 1RM.
is there some nice tool for
apstracted?@majkinetor commented on GitHub (Apr 27, 2023):
How the connections are stored and where :)
So we can add more store backend in the future, like git (great for team sharing without hassle of installing db) or simple JSON file or whatever.
But OK, you say you already designed it that way...
@majkinetor commented on GitHub (Apr 27, 2023):
OK, so we need then some internal log instead of intrusive popups? Or what are your thoughts?
@VShawn commented on GitHub (Apr 27, 2023):
I designed popups as a fallback error prompt mechanism. I added it to avoid situations where the database encounters exceptions (for example, when SQLite is set to read-only while 1RM is running in the background) and users cannot detect them.You see, this time the popup also helped me discover that 1RM would encounter exceptions when 'VPN goes down' :)
Normally, popups should not be triggered。the upstream error handling should handle all of the connection exception, so there should not be any intrusive situations. As long as I handle connection errors in a more appropriate way, popups would not appear. However, considering all kinds of exceptions is difficult, so I need to take some time to refactor the error handling program for adding, deleting, updating, and selecting. Then, I need draw a flowchart to help me sort out which error situations may occur: such as when the database disconnects while the user is editing or when the user tries to delete data after the database has disconnected.
@VShawn commented on GitHub (Apr 29, 2023):
I've remove the alert so you will not get the popups when 1RM is running in the background.
Adding the ability of offline database and automatic reconnection by periodic checking, testing is still needed.
It is being considered whether it is possible to detect changes in computer network and automatically reconnect after such changes occur.
@majkinetor commented on GitHub (Apr 29, 2023):
Just an update.
Today I don't see the remote database.
details
I went on Edit -> Test and it said Success, on the back, it blocked here:
details
On escape it goes back with it and ALT M stopped working
details
@VShawn commented on GitHub (Apr 29, 2023):
I think these were fixed after
github.com/1Remote/1Remote@930a8c0458@VShawn commented on GitHub (May 2, 2023):
here is how 1RM works with db offline now. I think this ticket is donw, you can get the latest nighly build to try it.
@majkinetor commented on GitHub (May 4, 2023):
I have a better proposition - UX lacks this way, as you never know when it will magically reapear, in 4 minutes and 59 seconds or in 1s or what happened at all.
2. Hide all items in it and show 'Reconnect now' button/option instead
3. Or better, show all items, becuase the fact that db is not accessible doesn't mean connection is and since you have all the data in local cache it can still be used even without remote db
@VShawn commented on GitHub (May 5, 2023):
a reconnect button is what I was going to do next, thanks to your blueprint I will follow your approach now : )
@VShawn commented on GitHub (May 7, 2023):
https://user-images.githubusercontent.com/10143738/236653315-411ef96e-1d06-4a44-b2b3-5e02b18ba4ef.mp4
I made the view, is that ok?
@majkinetor commented on GitHub (May 7, 2023):
Yeah, it looks nice.
With the large list you might not be able to see the group since it will be out of the view (which is what will happen all the time as nobody uses this who doesn't have a lot of them), did you make some UX on affected connections? At least edit's Save button should be disabled with adequate tooltip.
@VShawn commented on GitHub (May 8, 2023):
No not yet. maybe add a gray background for them?
sure thing
@VShawn commented on GitHub (May 8, 2023):
https://user-images.githubusercontent.com/10143738/236819470-5db60368-3161-48de-afe8-2b139d067801.mp4
manually reconnect is work now
@majkinetor commented on GitHub (May 8, 2023):
Looks good, but I wouldn't show reconnect if connection is OK, only if its not OK, because it seems iritating that some numbers are ticking and everything is fine.
@VShawn commented on GitHub (May 9, 2023):
it still WIP now, i did not create the display style for it yet, ticking will be hidden then if it connected.
@VShawn commented on GitHub (May 11, 2023):
I think most tasks of this ticket is done
check list:
https://github.com/1Remote/1Remote/assets/10143738/deba299d-4808-4434-b6f2-ce730d028e57
@majkinetor commented on GitHub (May 11, 2023):
How come items are still available upon disconnection, based on our discussion on #400.
But yes, large gray block is ugly. The problem is you may have a long list and database is not pinned at the top while you scroll so as soon as it goes out of screen you don't see status any more. You don't see status in launcher too (if its available there). Maybe something else is less intrusive like italic font for connection ...
But OK, that can be done later, this is good enough for now, lets call it a day.
@VShawn commented on GitHub (May 11, 2023):
Originally posted by @VShawn in https://github.com/1Remote/1Remote/issues/400#issuecomment-1543062001
Since MySQL is successfully connected, items are still available upon disconnection, I implement it this way for now, we can make changes after we have made a decision.
And I show a disconnected icon instead of the gray, much better now:)
@majkinetor commented on GitHub (May 11, 2023):
OK, not sure I understand your local cache problem then since it is already cached, just not in the accessible way :)
@VShawn commented on GitHub (May 11, 2023):
I think the main difference between us is that I believe that user must connect to MySQL at list once before they can use the data stored there, while you believe that it should be possible to use cached data from MySQL even if a connection hasn't been established since startup. Is that correct?
@majkinetor commented on GitHub (May 11, 2023):
I just noted that it could be possible that someone can send you the cached data, if that is what you mean. But anybody can do that NOW trivially easy:
3-5 minutes of work tops.
Contrary to this, we aknowledge that this is actually possible, document it and warn users, note that there is really nothing one can do to protect those data once the team member got access one way or another, and use it for additional features 😃
@majkinetor commented on GitHub (May 11, 2023):
OK, bulk duplicate is not possible now (it probalby should be), but export is there. Its even faster. I just exported my company database within 5 seconds.
@majkinetor commented on GitHub (May 11, 2023):
I tested new feature. I see you disabled edit, not the save button. What if I want to see server details? I thought you should disable Save button within connection editor.
This also makes it strange as you may not notice why it doesn't work. See this: I disconnect from VPN, edit doesn't work, I scroll a bit and KnowIT is out of screen, I minimize to tray, restore it, its still on the same position (maybe I was there to start with and couldn't see KnowIT reconnecting) I try editing and it doesn't work... why it doesn't work.
If KnowIT was pinned at the top and not scrolling, it would be visible that it reconnects, or if connections were marked somehow, or at least if save is disbled everything works and Save tooltip can be changed to notify you (or background color of editor can be changed to signify its in disconnected state). Maybe color of editor is the best... you will notice that for sure and it isn't affecting other things.
@majkinetor commented on GitHub (May 11, 2023):
There is a problem with current UX, the dissapearing/appearing message moves other icons :) So if you want to hunt for invisible + you may have hard time doing that.
@VShawn commented on GitHub (May 13, 2023):
Form this point, disable save button directly is not a good choice, since user may not know why the button is disabled. I would like to add a disconnected icon aside save button and do not disabled it.
then this would be better
https://github.com/1Remote/1Remote/assets/10143738/48d0c61b-32eb-4246-87a3-4f3b5ad0fcd6
@VShawn commented on GitHub (May 13, 2023):
You want a fix group header?
not sure for that, if the group header of the system's native control does not support fix on top, it may be necessary to completely customize a control. I have no enough experience in this aspect.
@majkinetor commented on GitHub (May 13, 2023):
Anything to inform us there is disconnect really. Here are some options, and you see what is easiest for you
details
details
details
Option 2 seems with least intrusion, not sure how visible it is. 1. kinda looks bad? 3. Looks OK, maybe a bit spammy? Maybe some less intrusive icon or just unicode text:
details
EDIT: Moved system tag to discussion https://github.com/1Remote/1Remote/discussions/416
My vote is for the system tag wired up also to availability detection (the screen bellow) that one could be able to invoke on selection without connecting at all (in parallel threads ofc). You manage it, its different color but behaves otherwise like normal tag. User can't change it.
details
@majkinetor commented on GitHub (May 14, 2023):
I see you replaced checkbox on latest version:
With Save icon we can consider this matter closed (although Launcher is left out but it was never a topic and I consider that minor hinderance)
@majkinetor commented on GitHub (May 14, 2023):
Actually, after more tests, it still doesn't work correctly:
@VShawn commented on GitHub (May 16, 2023):
I can not reproduce this maybe because I tested it by close MySQL since I don't have a VPN?
https://github.com/1Remote/1Remote/assets/10143738/e2651ef3-773f-4740-80b1-7fcc32c426b1
finally find it a UI bug, the counting down is 5min not 60s. I mistakenly set the UI to display seconds only, so it will automatically reconnect only after 5 timers of 60 seconds counting down have passed.
@VShawn commented on GitHub (May 16, 2023):
Oh?
Naturally, RAM is gone and with it my connectionsis fix now ?@majkinetor commented on GitHub (May 16, 2023):
Now it doesn't work at all, I am disconnected but it doesn't show:
details
No
details
Here, when I connected again there are no connections and still says I am disconnected. I had to restart to show them. Didn't wait 5 minutes to see what will happen but used manual connect.
@VShawn commented on GitHub (May 16, 2023):
Things are getting a bit strange. I didn't encounter the bug you mentioned while testing 1RM. I recorded a complete 5mi screen capture, which you can take a look.
https://github.com/1Remote/1Remote/assets/10143738/1b5c6bf8-6143-4d23-a376-56eeb4a77e32
I think it may be due to VPN that I am unable to reproduce the bug. I tried simulating network disconnection by turning the MySQL service on and off, but I couldn't test the situation where the VPN client was opened like you because I don't have an available VPN.
And manual connect button is not working on my end.
@majkinetor commented on GitHub (May 16, 2023):
You can't argue with the screenshot :) And it clearly was working before (talking about the icon).
@VShawn commented on GitHub (May 16, 2023):
No argue, I'm just confused about how to reproduce the bug.
@VShawn commented on GitHub (May 17, 2023):
i think this is the reason of the bug.
@VShawn commented on GitHub (May 17, 2023):
@majkinetor can you test it again? i shorten the reconect time to 60s to make it easier to test.
@majkinetor commented on GitHub (May 17, 2023):
Sure, later when I come home. U should put timer setings in json config.
@majkinetor commented on GitHub (May 17, 2023):
It works now, apart from connections gone when restarted, but you said you want it that way.
One thing seems buggy tho - it rechecks on 10s when OK, but on 60s when not ok
@majkinetor commented on GitHub (May 17, 2023):
Another thing is buggy in this version - clicking on database name doesnt always show/hide database as before. It works like this.
@VShawn commented on GitHub (May 18, 2023):
That's 2 different settings, I will then make it configurable
this is still WIP I found the expander dose not work after ordered by connection name desc, I accidentally pushed the code to Git while it half way done.
@majkinetor commented on GitHub (May 18, 2023):
Sort doesn't influence it.
Its overkill to have 2 settings IMO.
@VShawn commented on GitHub (May 18, 2023):
It does in previous version
You need to set a short time for checking data updates and a longer time for the database reconnection retry wait time. Trying to reconnect to the database every 10 seconds is overkill IMO
Anyway, these two settings can only be changed by eidt the json file at this moment.
@majkinetor commented on GitHub (May 18, 2023):
I didn't know the one is for data updates, I was thinking its also for availability but its natural that availability is consequence of checking for data, so yeah, that needs another timer configuration.
And it should IMO stay that way, we don't need advanced options in the configuration. We could add a button to open json like for example vscode or SumatraPDF or windows terminal.