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

Validate Runtimes notleader not called on all transitions #4297

Closed
anthony-murphy opened this issue Nov 10, 2020 · 3 comments · Fixed by #4785
Closed

Validate Runtimes notleader not called on all transitions #4297

anthony-murphy opened this issue Nov 10, 2020 · 3 comments · Fixed by #4785
Assignees
Labels
area: runtime Runtime related issues bug Something isn't working manual testing Problems around manual testing/validation
Milestone

Comments

@anthony-murphy
Copy link
Contributor

anthony-murphy commented Nov 10, 2020

notleader should fire in all cases where the client is no longer the leader. inlcuding:

  • disconnect
  • close
  • dispose
  • readonly
@anthony-murphy anthony-murphy added the bug Something isn't working label Nov 10, 2020
@ghost ghost added the triage label Nov 10, 2020
@curtisman curtisman added the area: runtime Runtime related issues label Nov 10, 2020
@curtisman curtisman added this to the December 2020 milestone Nov 10, 2020
@ghost ghost removed the triage label Nov 10, 2020
@vladsud
Copy link
Contributor

vladsud commented Nov 10, 2020

It is definitely called on disconnect:

        this.runtime.on("disconnected", () => {
            if (this.runtime.attachState !== AttachState.Detached) {
                this.clearRunningTasks();
            }
        });

Close causes dispose, so the only thing missing is dispose.

@anthony-murphy
Copy link
Contributor Author

anthony-murphy commented Nov 11, 2020

Good to hear. I was unsure of the behavior, and FFX was saying it wasn't working as they expected. I've renamed, as this is more about ensure that all transitions are covered. Ideally, a consumer interested in leader status shouldn't really need to care about other state transitions

@anthony-murphy anthony-murphy changed the title Runtimes: notleader not called on disconnect, close, or dispose Validate Runtimes notleader not called on all transitions Nov 11, 2020
@curtisman curtisman added the manual testing Problems around manual testing/validation label Dec 1, 2020
@curtisman
Copy link
Member

Please add test for all these code path.

@curtisman curtisman modified the milestones: December 2020, January 2021 Dec 1, 2020
@curtisman curtisman self-assigned this Jan 11, 2021
curtisman added a commit that referenced this issue Jan 12, 2021
Fix #4297
- the LocalDocumentDeltaConnection.close need to disconnect the client.
- OpProcessController need to keep track of disconnecting client and expecting Leave messages.

Other changes:
- Don't lint for the build task that is run before we launch the debugging session.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: runtime Runtime related issues bug Something isn't working manual testing Problems around manual testing/validation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants