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

INvalid character written to tunnel json file #108

Closed
ikidd opened this issue Nov 30, 2021 · 19 comments
Closed

INvalid character written to tunnel json file #108

ikidd opened this issue Nov 30, 2021 · 19 comments
Labels
bug Something isn't working

Comments

@ikidd
Copy link
Contributor

ikidd commented Nov 30, 2021

JSON datafile sometimes will have invalid character at end and dashboard won't read properly, hangs going into that tunnels configuration page.

I added a new peer via phone browser, oddly it seemed to pick up all the info for the previously created and saved peer. I fixed the IP address and other fields, then when I saved this last peer, it stopped responding. Could get into other tunnel's config pages, but not the one that I was in when I saved that peer.

Went to filesystem, opened that tunnel's json file. When it opened in a json parser like Firefox, it would give an error parsing json at Line 1, Columns 2929. At that location in the file, there were a couple unknown characters: "^@" instead of a curly brace to close the first brace in the file. Removed and replaced with curly brace, reloaded wg-dashboard service and all was fine.

@ikidd ikidd added the bug Something isn't working label Nov 30, 2021
@donaldzou
Copy link
Owner

I will look into this, it seems like the way I let the dashboard to write into the database isn't right (since i wrote the functions about a year ago, and when read them now I always ask myself why I wrote it in this way lol). I will look into this when I start building the next release. Thanks alot!!

@ikidd
Copy link
Contributor Author

ikidd commented Dec 22, 2021

Happened again when saving a peer, saving process hung and had a bunch of invalid character (^@) at the end. Fixed the missing part and put }}} on the end, all good.

Any direction where I can work to fix this? This is really the only showstopper issue I have with this, currently using it in production otherwise, but I have to make a terminal available to fix this when it happens which is limiting me.

@donaldzou
Copy link
Owner

I was trying to figure out why... could you uninstall tinydb from pip and reinstall it? It might be a bug from the tinydb library. Could you also tell me was there any special characters that you inputed with the new peer? Thanks!

@ikidd
Copy link
Contributor Author

ikidd commented Dec 22, 2021

It's pretty intermittent. Only thing special was that it was a two word name "Raymond Camera" with a space. But I've done others with spaces successfully before, even another peer on this particular tunnel. And where it failed in the json was not at the space, it was a few characters off the end of the endpoint hostname.

But I fixed the json and tried 3 times with the space, and then it went when I dropped the space. So I'm going to go with the "sometimes it's a space that does it" theory for now. I can try doing the tinydb thing but it's very intermittent, it's only happened twice.

@donaldzou
Copy link
Owner

I will try another setting with the database to see if it will occur. I'll let you know when I pushed the changes.

@donaldzou
Copy link
Owner

I'm guessing it might do with when it is writing and reading the database at the same time, i don't think tinydb have concurrency reading/writing implemented.

@ikidd
Copy link
Contributor Author

ikidd commented Dec 23, 2021

YOu're thinking along these lines?

msiemens/tinydb#113

Should you not be doing a db.close for every db= statement before passing out of the function if this is the case? Because I see many, many places where the db is just left hanging as far as I can follow the code. There's a bunch of things like case statements that return out of the function; even if the db.close is at the end of the function, it would never get hit. If I were worried about leaving a db connection open, I'd open the db, pull whatever queries I need into variables for the function, and close immediately, reopening temporarily for anything that needs to be written in.

I can go through and try to identify these spots. the functions at 460 and 473 are two that I found just browsing through.

I'm no python guy, I could be talking out my ass on this.

@donaldzou
Copy link
Owner

Yes exactly, thanks for digging thru the repository. I'm trying to close everywhere i opened and lock the file when i read it.

@ikidd
Copy link
Contributor Author

ikidd commented Dec 23, 2021

I don't know much about the tinydb, but maybe an completely inelegant and ugly solution would be putting a db.close before every db= statement just to catch an inadvertent hanging connection. It's gross, but unless it kicks you out of the function if there isn't an open db, it would work and would be easy to find and replace in.

Again, I have no clue how to debug a web application like this, so perhaps that's a dumb idea.

@donaldzou
Copy link
Owner

I'm closing every opened db, it would be the best to migrate this to sqlite or mongodb, but with sqlite i need to change many of my codes, and with mongodb the user need run mongodb by itself, so the project is no longer lightweight and easy to use/install

@ikidd
Copy link
Contributor Author

ikidd commented Dec 23, 2021

I like the json format of the tinydb, it's easy to fix at least, and this is not a huge db consumer application. I can see why you'd use it. I might be noticing some speed issues now that I have some tunnels with 10+ peers in them.

Thinking about it, you'd just need to do the kludgy fix on db= operations where you were about to write something (like that save operation), because every other db operation would be a read and wouldn't care if it was open already. In fact, besides Settings, that's probably the only write operation in the application. Edit: I take that back, I see a bunch more writes.

@donaldzou
Copy link
Owner

Yeah, that's the reason why I kept this form as the database. Regarding to the speed issue, i'm thinking it might be something to do with the html and GUI that is causing the issue, because just by loading the data from the database will only cost ~0.01s to 0.04s with 100+ peers... that is the next thing i will look into it.

@ikidd
Copy link
Contributor Author

ikidd commented Dec 23, 2021

I can fork it and try to fix some of the ones I find if you want. Don't want to duplicate effort and it might be just as much effort for you to review my PR than it is to just go through and fix them yourself.

@donaldzou
Copy link
Owner

I just got the same issue...

  File "/usr/lib/python3.8/json/__init__.py", line 357, in loads
    return _default_decoder.decode(s)
  File "/usr/lib/python3.8/json/decoder.py", line 337, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
  File "/usr/lib/python3.8/json/decoder.py", line 353, in raw_decode
    obj, end = self.scan_once(s, idx)
json.decoder.JSONDecodeError: Expecting ',' delimiter: line 1 column 3217 (char 3216)

I opened 3 tabs, and used 1 to add more peers and caused the issue

@donaldzou
Copy link
Owner

I can fork it and try to fix some of the ones I find if you want. Don't want to duplicate effort and it might be just as much effort for you to review my PR than it is to just go through and fix them yourself.

For sure! I appreciated!

@donaldzou
Copy link
Owner

donaldzou commented Dec 24, 2021

@ikidd Hi, I just pushed a branch https://github.com/donaldzou/WGDashboard/tree/v3.0-Slow-Load-Fix should fix the slow loading problem that you are occurring and should fix the database problem🤣 . It is because Flask need to generate the QRcode for each peer when loading, I disabled that and make it generate the qrcode when user click the icon. Could you please try it when you're free and let me know if it fixed the problem. Thanks!

@ikidd
Copy link
Contributor Author

ikidd commented Dec 24, 2021

I'm a little scared of the change to using venv that's been introduced, so I might not get around to this until I clear that mental hurdle and understand what virtual environments are all about.

@donaldzou
Copy link
Owner

donaldzou commented Dec 25, 2021

I'm a little scared of the change to using venv that's been introduced, so I might not get around to this until I clear that mental hurdle and understand what virtual environments are all about.

You can check this out https://www.freecodecamp.org/news/python-virtual-environments-explained-with-examples/. Basically have a copy of your Python interpreter into the dashboard directory and run directly from it. So it could isolate other package you installed with your default library.

Merry Christmas!

@donaldzou
Copy link
Owner

I think.... I think.. I solved this issue completely (I think) By using a thread safe database (SQLite) in the next release, and should be good to go. #126

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants