Skip to content
This repository has been archived by the owner on Dec 22, 2020. It is now read-only.
This repository has been archived by the owner on Dec 22, 2020. It is now read-only.

JS error handling [improvements needed] #30

Closed
rocketinventor opened this issue Feb 24, 2017 · 7 comments
Closed

JS error handling [improvements needed] #30

rocketinventor opened this issue Feb 24, 2017 · 7 comments

Comments

@rocketinventor
Copy link
Contributor

rocketinventor commented Feb 24, 2017

I have noticed that if the server responds with any type of error, the client demobot.html freaks out. When this happens, no move is processed for white and the client switches to playing white for one (1) move. At this point, the player can only play in areas that white is allowed to play in, and locally, their moves count for white. However, the moves are still recorded and sent to the server as moves made by black.

There is currently no way to update the board with the server and there is no way (that I know of) to undo the last move (easily). The only way to avoid this issue (at this point) is to hold off actually making the move until the server resolves the move (successfully). Another option is that every time a move is made, a "snapshot" could be recorded before it is processed (and restored if it causes an error).

This lack of error handling is the partial cause for the symptoms described in #29, #26, and #27.

@macfergus
Copy link
Collaborator

I propose splitting the /predict HTTP command into two operations: one to play the human move, and a second to generate the bot move

Then the client flow could be:

  1. Make the server request to apply the human move
  2. Wait for request to finish successfully (should be fast)
  3. Update client state and draw the human move
  4. Make a server request for a bot move (could be slow, with a more sophisticated bot engine)
  5. Update client state and draw bot move

(Optimistic update with error handling might be nice for a production-quality client, but for the demo client I think blocking is fine)

@rocketinventor
Copy link
Contributor Author

While splitting the request may be a good idea, I don't think that it would help solve the problem. If the bot has an error, we will still have a problem. Also, the additional level of complexity is not worth it. I'd rather have a separate request for getting a copy of the game from the server.

If we did go the route of splitting the request, it might make more sense to use web-sockets.
The client flow would then look like this:

Upon starting:

The client can either choose to join an in-progress game or to start a new one.

  1. Open socket connection with server
    a. Optionally: collect more data (i.e. username)
  2. Check if a game is in progress. Then do one of the following:
    a. Ask for a copy of the game data
    b. Start a new game instance (with default settings) or
    c. Notify server of non-default_preferences_ (see Better UI: Choose color, komi, handicap, start new game, etc. #6 for ideas), then start a new game instance
  3. Join the chosen game session/instance
  4. Receive copy of current game state (if in progress)

During a game session:

  1. Update server with human move color and coordinates
  2. Wait for a confirmation from the server
  3. Draw human move
  4. Wait for the predicted move
  5. Update client state and draw move

The benefit of this route is that the server could be programmed to mirror the current game across all open web clients. In this scheme, step 1 is optional! All the other steps (2 - 6) can happen concurrently on a (hypothetically) infinite amount of clients with one or all of them able to pick the next move.

@rocketinventor
Copy link
Contributor Author

I think the best route to go is to draw the stone but not run jboard.setType(playNext.captures, JGO.CLEAR) (clear opponent's stones) until a success is recorded from the server. If there is an error, the stone should be removed.

@rocketinventor
Copy link
Contributor Author

rocketinventor commented Feb 26, 2017

It's worth noting that moves are getting sent to the server, even if they are invalid.

Client-side log:
image

If the server doesn't reject it (i.e. a suicide or immediate ko retake), it will return another move. However, the client will not draw the move because it is invalid. This is one of the contributing issues to #27.
image

This should be easy to fix.

@rocketinventor
Copy link
Contributor Author

rocketinventor commented Feb 26, 2017

Additionally, the next move after an invalid (but server-accepted) move is usually ignored but, sometimes it is evaluated (and even drawn) as a black move.

Please see the below diagram below. The black box is where I attempted to play and the white box is the play that the server responded with.

image
As you can see, both moves were accepted by the server but rejected by the client.
image

I have also had times where a black stone actually did get drawn
The rest of the time, the response from the server is silently dropped.

@macfergus
Copy link
Collaborator

I think the best route to go is to draw the stone but not run jboard.setType(playNext.captures, JGO.CLEAR) (clear opponent's stones) until a success is recorded from the server. If there is an error, the stone should be removed.

Yes this is a good plan.

Just to clarify what I was saying about splitting the /predict call:

  1. The client should check move validity with the server. If the server rejects a move, the client should not update the board (and instead should re-sync the game state, probably).
  2. But the bot may be slow to generate its next move. It would be a bad experience if the human player had to wait 10 - 20 seconds to see their move on the board. (Thinking ahead to a more sophisticated version of the bot.)
  3. Splitting the request means we can confirm the move quickly, and the user will see the board updated after just a few ms.
  4. Splitting the request makes the HTTP protocol more similar to GTP. (In fact, we could consider some kind of GTP-over-HTTP scheme.)

Using web sockets to sync state across multiple browser tabs seems like overkill to me, but if that's what interests you, by all means go for it :)

Supporting full sync of game state from the server is a good idea in any case.

@rocketinventor
Copy link
Contributor Author

I am going to go ahead and close this issue due to the improvements of pull request #38 which address the actual issue of poor client side handling. The rest of this thread, which is about sync mechanisms, should be given its own issue for discussion as it is off topic.

If anyone else notices new problems with the javascript error handling, they should feel free to re-open this thread.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants