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

6.0 Feedback: UI redirect after a robot rename will be wrong if name has conflict #10709

Closed
mcous opened this issue Jun 9, 2022 · 0 comments
Closed
Assignees
Labels
6.0-feedback Feedback for the 6.0 release

Comments

@mcous
Copy link
Contributor

mcous commented Jun 9, 2022

Overview

With #10706, our robot renaming UI has a "new" edge case to worry about. TL;DR: I propose that the app redirect to /devices after a robot rename rather than /devices/:newName. Reasoning below.

There are two possible sources for a mDNS naming conflict:

  1. Another OT-2 with the same name
  2. Some network device that isn't an OT-2 with the same name

From a very technical standpoint, name conflicts are completely asynchronous events. What that means for us right now is that the response body from POST /server/name cannot be trusted. The order of events during a name-change-with-conflict looks like:

  1. App issues POST /server/name {"name": "newname"}
  2. Server responds 200 with {"name": "newname"}
  3. App redirects to UI route /devices/newname
  4. Avahi tells server that newname has a conflict, and it cannot advertise under that name
    • This will happen within a few seconds of the request
  5. Server renames self newnameNum2
  6. Discovery client picks up robot at newnameNum2
  7. Subsequent requests to GET /server/name (or health, etc.) return newnameNum2

Current Behavior

By the time the rename is done and the Discovery Client has picked up the new name, the correct UI route for the renamed robot will be /devices/newnameNum2. However, the app will be displaying /devices/newname, and it will have no way to associate newname2 with the rename action it just took.

Depending on the source of the conflict, the UI route /devices/newname will be incorrect in one of two ways:

  1. Conflict source: another OT-2 already named newname
    • The settings page of the OT-2 named newname, which is not the robot the user just renamed
  2. Conflict source: some network device that isn't an OT-2 but is named newname
    • A mostly blank robot settings page, because there is no robot named newname, that will remain blank until the user navigates away
    • Or, in the off chance there's a historical OT-2 that used newname, the settings page for an unavailable robot called newname

It's worth noting that a rename without a conflict can look a lot like (2) at the moment. After the rename, the settings page may go blank for a second or two until the Discovery Client picks up on the new name.

Expected Behavior

I think a more reliable experience until #10689 is resolved - for both regular renames and conflicted renames - would be to redirect the user to the top-level /devices page after a rename instead of trying to keep them on the same robot's page.

It's still not a great experience, but I think it's less risky than potentially dropping the user into the settings page of a different robot.

Steps To Reproduce

Once #10706 is merged, rename a robot to something with a conflict intentionally.

Operating system

No response

Robot setup or anything else?

The longer term issue / fix is tracked in #10689

@mcous mcous added the 6.0-feedback Feedback for the 6.0 release label Jun 9, 2022
@koji koji self-assigned this Jun 9, 2022
koji added a commit that referenced this issue Jun 9, 2022
koji added a commit that referenced this issue Jun 10, 2022
koji added a commit that referenced this issue Jun 22, 2022
This PR is for robot rename function and this PR will be used by #10723 to retrieve the new robot
name

partially fix #10709
@bndo bndo closed this as completed Jun 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.0-feedback Feedback for the 6.0 release
Projects
None yet
Development

No branches or pull requests

5 participants