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

[1] v2.0.170829 unable to add nodes #337

Closed
abitmore opened this issue Aug 30, 2017 · 21 comments
Closed

[1] v2.0.170829 unable to add nodes #337

abitmore opened this issue Aug 30, 2017 · 21 comments
Assignees
Labels
[3] Bug Classification indicating the existing implementation does not match the intention of the design
Milestone

Comments

@abitmore
Copy link
Member

abitmore commented Aug 30, 2017

In settings->access, it jumps to main page when clicking "add node".

@wmbutler
Copy link
Contributor

Agreed. @calvinfroedge

@wmbutler wmbutler added the [3] Bug Classification indicating the existing implementation does not match the intention of the design label Aug 30, 2017
@wmbutler
Copy link
Contributor

wmbutler commented Aug 30, 2017

Calvin, I tested this at bitshares.org/wallet and the bug report is accurate. I've added it into a patch milestone because this should be a hotfix for release today. This might have gotten broken as part of the merge process because I remember this was working when I initially tested it.

@btsfav
Copy link

btsfav commented Aug 30, 2017

@wmbutler it's also bugged in the light wallet(windows). if you click access the main screen gets stuck

don't you test this stuff?

@wmbutler
Copy link
Contributor

wmbutler commented Aug 30, 2017

@btsfav Yes, we saw that and it's being addressed. Looks to me as if it happened during the merge because this was tested on the feature branch.

@btsfav
Copy link

btsfav commented Aug 30, 2017

just tested in webwallet... the bugs seem slightly different?

see light wallet:

@wmbutler @svk31

@calvinfroedge
Copy link
Contributor

calvinfroedge commented Aug 30, 2017

@abitmore @btsfav

There seem to be a few different issues here...

  1. Electron issue being able to add nodes was resolved...apparently electron handles links slightly differently than a browser does and I could not use an anchor tag for the link. PR: Resolve linking issue in electron #340

  2. I cannot reproduce the error clicking the "Access" tab. However, I was not able to use the bitshares.org/wallet at all until clearing local storage.

  3. Web wallet seems to have issues with getting server latencies. As the ping logic was not modified, this would be an existing issue.

@abitmore
Copy link
Member Author

abitmore commented Aug 30, 2017

@btsfav try delete everything under Local Storage -> file:// . #336 (need to open developer console first) @calvinfroedge I can't reproduce this either, but got several reports from users. Not sure why.

@calvinfroedge
Copy link
Contributor

Updated build:

https://drive.google.com/file/d/0BzvuEYutkmtdTHFYbWRuWkFxX2s/view?usp=sharing

sha1: 79e65edf1ef57f32768501031ca85bc6d565fcb2
sha256: f15892e3edd1328343f7f5a89a51b92a1ee41ef9653de5c0d3da7d1e244e5c96

@btsfav
Copy link

btsfav commented Aug 30, 2017

@calvinfroedge

maybe this helps

@abitmore

not sure what you want me to do

@calvinfroedge
Copy link
Contributor

@btsfav Open your console and type localStorage.clear(). I'm trying to support you on this, but did not create the issue you're experiencing.

@btsfav
Copy link

btsfav commented Aug 30, 2017

@calvinfroedge
localStorage.clear()
undefined

edit: nvm seems to work
edit2: shows all nodes as DOWN though :)

@calvinfroedge
Copy link
Contributor

calvinfroedge commented Aug 30, 2017

edit2: shows all nodes as DOWN though :)

@btsfav I added that as 3rd point above. It does not seem to be related to the changes I made:

Web wallet seems to have issues with getting server latencies. As the ping logic was not modified, this would be an existing issue.

This works in the electron version and in local builds. The ping logic was not changed, so I imagine this issue is environment specific and was pre-existing. We can address it as a separate issue but we will need to do some additional work to set up effective testing in every environment bitshares is being deployed.

As you can see, I am seeing pings in electron build and also in local development (web):

49-ex-2

@abitmore
Copy link
Member Author

@btsfav
f7c2364c-ad6d-474e-ab9c-2f3f05b9d5b8

@btsfav
Copy link

btsfav commented Aug 30, 2017

@calvinfroedge after I selected "closest..." nodes are displayed correctly

@abitmore thanks, worked via the console cmd

@calvinfroedge
Copy link
Contributor

@btsfav Ok, good to know that selecting closest fixes, I will check to see if I can figure out why.

@wmbutler wmbutler changed the title v2.0.170829 unable to add nodes v2.0.170829 unable to add nodes (localstorage issue with new version) Aug 30, 2017
@wmbutler wmbutler changed the title v2.0.170829 unable to add nodes (localstorage issue with new version) [4] v2.0.170829 unable to add nodes (localstorage issue with new version) Aug 30, 2017
@wmbutler
Copy link
Contributor

When a new version of the client is released, browser localstorage can hold onto information that causes unexpected behavior regarding node status and selection. The user can delete localstorage manually, but a better system would be for the code to compare it's version, possible a version in package.json to a value place in localstorage. If the versions match, leave localstorage untouched. If values are different, clear localstorage.

@wmbutler wmbutler added this to the 170914 milestone Aug 30, 2017
@calvinfroedge
Copy link
Contributor

@wmbutler It would probably be a good idea to include the patch / PR I already issued for fixing the electron issue now rather than waiting till 9/14.

@wmbutler
Copy link
Contributor

Agreed... @svk31

@svk31
Copy link
Contributor

svk31 commented Aug 30, 2017

Clearing local storage is a bad idea, lots of useful stuff is stored, like all the user's settings, possibly deposit addresses etc.

@wmbutler
Copy link
Contributor

But we do need a way to resolve this issue that doesn't require user intervention, So I'm keeping it open for discussion.

@wmbutler wmbutler changed the title [4] v2.0.170829 unable to add nodes (localstorage issue with new version) [1] v2.0.170829 unable to add nodes (localstorage issue with new version) Aug 30, 2017
@wmbutler
Copy link
Contributor

I'll simplify this issue as adding a node and we can tackle localstorage issue elsewhere.

@wmbutler wmbutler changed the title [1] v2.0.170829 unable to add nodes (localstorage issue with new version) [1] v2.0.170829 unable to add nodes Aug 30, 2017
@wmbutler wmbutler added this to the 170830 milestone Aug 30, 2017
@wmbutler wmbutler removed this from the 170914 milestone Aug 30, 2017
svk31 added a commit that referenced this issue Aug 30, 2017
@wmbutler wmbutler modified the milestones: 170830, 170914 Aug 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[3] Bug Classification indicating the existing implementation does not match the intention of the design
Projects
None yet
Development

No branches or pull requests

5 participants