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

Issues after browser refresh #143

Closed
cesarcarc opened this issue Apr 28, 2020 · 6 comments · Fixed by #178
Closed

Issues after browser refresh #143

cesarcarc opened this issue Apr 28, 2020 · 6 comments · Fixed by #178
Assignees
Labels
bug Something isn't working client high priority

Comments

@cesarcarc
Copy link
Contributor

Two scenarios were identified. This has been tested on Chrome (Version 81.0.4044.122 (Official Build) (64-bit)). Tests were performed with version 0.5.1 and 0.6.0 (after the release of cse-core).


Scenario 1:

Setup

  • Run cse-server-go, load house demo, press play

Action 1:

  • While running, have the "simulation status" page open. Press refresh

Result 1:

  • Models disappear (Not ok)

Action 2:

  • Click on a trend and refresh browser

Result 2:

  • Models are now displayed (ok)

Scenario 2:

Setup

  • Run cse-server-go (start it again), load house demo, press play

Action:

  • While running, have one of the trends page open. Press refresh

Result:

  • Models don't disappear (ok)
  • Trend freezes (not ok)
  • A lot of data is dumped to the terminal (not ok)

Depending on the page you are when you hit refresh, the models will disappear or not (but that is only applicable for the first refresh).

@flakstad
Copy link
Contributor

flakstad commented Apr 29, 2020

Unable to reproduce scenario 1 in Firefox or Chrome against cse-server-go-0.6.0

@cesarcarc cesarcarc transferred this issue from another repository May 18, 2020
@cesarcarc cesarcarc added bug Something isn't working client labels May 18, 2020
@flakstad
Copy link
Contributor

Scenario 1:
Could it be that the reagent component for the models in the side menu is of form #1, ie the component returns a vector and the models are dereferenced in the let-binding? If so, the problem might be solved by using the reagent form #2, returning a fn and dereferencing the subscription inside the fn.

@cesarcarc cesarcarc self-assigned this May 26, 2020
@cesarcarc
Copy link
Contributor Author

Scenario 1:
Could it be that the reagent component for the models in the side menu is of form #1, ie the component returns a vector and the models are dereferenced in the let-binding? If so, the problem might be solved by using the reagent form #2, returning a fn and dereferencing the subscription inside the fn.

For scenario 1:
The problem doesn't seem to be in the client side. When I hit refresh, the client sends a "get-module-data" via websocket. The return message comes without the module-data. It is almost as the server misses the command every now and then. I will investigate the server side.

@cesarcarc
Copy link
Contributor Author

@eidekrist, I believe this is connected to:
ingesolvoll/kee-frame#65

I have debugged a little further on the server side. It is just some timing situation. If you hit refresh a second time, it might work. What is the best approach? to send a second time the command (from the client side) or to fix the server such that if it fails to write the feedback, it tries again? The latter seems better.

@eidekrist
Copy link
Member

That bug in kee-frame is supposed to be fixed. If it was not fixed, then there would be no "get-module-data" message in the network log.

I tried reproducing, and it seems the "get-module-data" command is sent to the server, but there is no reply with the correct stuff. The best approach is to debug the server code and see what really happens with the initial "get-module-data" command instead of patching it up.

Bonus info; when the socket connection closes (refresh browser) the server dumps the current state to the console. It is easy to get confused by that. I'd love to take a deeper look but I have to wait until next week.

@eidekrist eidekrist changed the title Issues after browser refresh [client] Issues after browser refresh Jun 2, 2020
@eidekrist
Copy link
Member

Split off scenario #2 as a separate issue, see #177.

eidekrist added a commit that referenced this issue Jun 3, 2020
eidekrist added a commit that referenced this issue Jun 3, 2020
* #175 Clear all stored X and Y trend values when changing active trend.

* #175 Fetch initial trend data faster when navigating to a plot.

* #143 Remove trend loop.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working client high priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants