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

Handle devtools clients disconnecting better #27512

Merged
merged 2 commits into from Aug 6, 2020

Conversation

jdm
Copy link
Member

@jdm jdm commented Aug 5, 2020

These changes fix many opportunities for the devtools server to panic if a client disconnects early. Now that it's easy to have multiple clients connected simultaneously, this also makes it easier for server code to distinguish between different clients and ensure that one client disconnecting doesn't affect another client that's still connected.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix Panic after closing devtools panel #27509
  • These changes do not require tests because the devtools server is a wild west.

@jdm
Copy link
Member Author

jdm commented Aug 5, 2020

r? @asajeffrey

@highfive highfive assigned asajeffrey and unassigned nox Aug 5, 2020
@Manishearth
Copy link
Member

Manishearth commented Aug 6, 2020

@bors-servo r+

@bors-servo
Copy link
Contributor

bors-servo commented Aug 6, 2020

📌 Commit 7011273 has been approved by Manishearth

@bors-servo
Copy link
Contributor

bors-servo commented Aug 6, 2020

Testing commit 7011273 with merge 7b0971e...

bors-servo added a commit that referenced this issue Aug 6, 2020
Handle devtools clients disconnecting better

These changes fix many opportunities for the devtools server to panic if a client disconnects early. Now that it's easy to have multiple clients connected simultaneously, this also makes it easier for server code to distinguish between different clients and ensure that one client disconnecting doesn't affect another client that's still connected.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #27509
- [x] These changes do not require tests because the devtools server is a wild west.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
@bors-servo
Copy link
Contributor

bors-servo commented Aug 6, 2020

💔 Test failed - status-taskcluster

@Manishearth
Copy link
Member

Manishearth commented Aug 6, 2020

./components/devtools/lib.rs:130: derivable traits list is not in alphabetical order

@jdm
Copy link
Member Author

jdm commented Aug 6, 2020

@bors-servo r=Manishearth

@bors-servo
Copy link
Contributor

bors-servo commented Aug 6, 2020

📌 Commit f4915ef has been approved by Manishearth

bors-servo added a commit that referenced this issue Aug 6, 2020
Handle devtools clients disconnecting better

These changes fix many opportunities for the devtools server to panic if a client disconnects early. Now that it's easy to have multiple clients connected simultaneously, this also makes it easier for server code to distinguish between different clients and ensure that one client disconnecting doesn't affect another client that's still connected.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #27509
- [x] These changes do not require tests because the devtools server is a wild west.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
@bors-servo
Copy link
Contributor

bors-servo commented Aug 6, 2020

Testing commit f4915ef with merge ffa8417...

@bors-servo
Copy link
Contributor

bors-servo commented Aug 6, 2020

💔 Test failed - status-taskcluster

@jdm
Copy link
Member Author

jdm commented Aug 6, 2020

@bors-servo
Copy link
Contributor

bors-servo commented Aug 6, 2020

Testing commit f4915ef with merge e16829f...

bors-servo added a commit that referenced this issue Aug 6, 2020
Handle devtools clients disconnecting better

These changes fix many opportunities for the devtools server to panic if a client disconnects early. Now that it's easy to have multiple clients connected simultaneously, this also makes it easier for server code to distinguish between different clients and ensure that one client disconnecting doesn't affect another client that's still connected.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #27509
- [x] These changes do not require tests because the devtools server is a wild west.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
@bors-servo
Copy link
Contributor

bors-servo commented Aug 6, 2020

💔 Test failed - status-taskcluster

@jdm
Copy link
Member Author

jdm commented Aug 6, 2020

@bors-servo retry

@bors-servo
Copy link
Contributor

bors-servo commented Aug 6, 2020

Testing commit f4915ef with merge 5cbcf90...

bors-servo added a commit that referenced this issue Aug 6, 2020
Handle devtools clients disconnecting better

These changes fix many opportunities for the devtools server to panic if a client disconnects early. Now that it's easy to have multiple clients connected simultaneously, this also makes it easier for server code to distinguish between different clients and ensure that one client disconnecting doesn't affect another client that's still connected.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #27509
- [x] These changes do not require tests because the devtools server is a wild west.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
@bors-servo
Copy link
Contributor

bors-servo commented Aug 6, 2020

💔 Test failed - status-taskcluster

@jdm
Copy link
Member Author

jdm commented Aug 6, 2020

@bors-servo retry

@bors-servo
Copy link
Contributor

bors-servo commented Aug 6, 2020

Testing commit f4915ef with merge 6a7e9ff...

@bors-servo
Copy link
Contributor

bors-servo commented Aug 6, 2020

☀️ Test successful - status-taskcluster
Approved by: Manishearth
Pushing 6a7e9ff to master...

@bors-servo bors-servo merged commit 6a7e9ff into servo:master Aug 6, 2020
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants