Skip to content

Add WebSocketException and support for WS handlers#1263

Merged
Kludex merged 20 commits intomasterfrom
feat/websockets-exception
Sep 5, 2022
Merged

Add WebSocketException and support for WS handlers#1263
Kludex merged 20 commits intomasterfrom
feat/websockets-exception

Conversation

@Kludex
Copy link
Owner

@Kludex Kludex commented Aug 11, 2021 •
edited
Loading

Missing

image

  • Check how this PR works interacting with the server.

@Kludex Kludex mentioned this pull request Dec 6, 2021
Closed
@aminalaee aminalaee mentioned this pull request Jan 27, 2022
Closed
@adriangb
Copy link
Contributor

adriangb commented Jan 27, 2022

@Kludex just fyi I updated this branch and fixed all issues

@Kludex
Copy link
Owner Author

Kludex commented Jan 27, 2022

don't worry :)

@adriangb adriangb added the feature New feature or request label Feb 2, 2022
@nameer
Copy link

nameer commented Feb 28, 2022

Any updates?

nameer
nameer reviewed Feb 28, 2022
View reviewed changes
@brunopcarv
Copy link

brunopcarv commented Mar 8, 2022

Any updates or help needed on this?

@obonyojimmy
Copy link

obonyojimmy commented Apr 14, 2022

Any updates on this ?

@Kludex
Copy link
Owner Author

Kludex commented Apr 14, 2022

No.

@Kludex
Copy link
Owner Author

Kludex commented Apr 21, 2022

Update here: I'll be working on this on the following weeks. If my memory doesn't fail me, this PR should be completed, but I need to implement something (which I don't know what, I need to check) on uvicorn.

@Kludex Kludex added this to the Version 0.2x.x milestone Apr 21, 2022
@Kludex Kludex modified the milestones: Version 0.21.0, Version 0.22.0 May 5, 2022
@Kludex Kludex marked this pull request as ready for review May 10, 2022 05:45
@Kludex
Copy link
Owner Author

Kludex commented May 10, 2022

I've rebased the PR, add the reason field on the WebSocketException, and updated the PR description.

@Kludex Kludex added the websocket WebSocket-related label May 10, 2022
@Kludex Kludex self-assigned this May 10, 2022
@Kludex Kludex modified the milestones: Version 0.21.0, Version 0.22.0 Jun 12, 2022
@nameer
Copy link

nameer commented Aug 12, 2022

Any update on this one?

@Kludex
Copy link
Owner Author

Kludex commented Aug 12, 2022

No.

@Kludex
Copy link
Owner Author

Kludex commented Aug 12, 2022

I'm going to work on this.

@Kludex Kludex modified the milestones: Version 0.22.0, Version 0.21.0 Aug 12, 2022
@Kludex Kludex changed the title Add WebSocketException and support for WS handlers Add WebSocketException and support for WS handlers Aug 13, 2022
@Kludex
Copy link
Owner Author

Kludex commented Aug 13, 2022

This PR is completed.

@tomchristie Would you mind checking it?

Considering the comment you made (described in the description), I've removed the logic from the ServerErrorMiddleware. Instead, I've created a follow-up on #22, do you think that makes sense?

@Kludex Kludex requested a review from a team August 13, 2022 14:17
@Kludex Kludex requested a review from lovelydinosaur August 18, 2022 06:00
@Kludex Kludex mentioned this pull request Aug 18, 2022
Merged
8 tasks
tiangolo
tiangolo approved these changes Aug 18, 2022
View reviewed changes
Copy link
Contributor

@tiangolo tiangolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🚀

lovelydinosaur
lovelydinosaur approved these changes Sep 5, 2022
View reviewed changes
Copy link
Contributor

@lovelydinosaur lovelydinosaur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic, yes.

@Kludex
Copy link
Owner Author

Kludex commented Sep 5, 2022

Thanks @tomchristie @tiangolo 🎉

@Kludex Kludex merged commit d525431 into Kludex:master Sep 5, 2022
aminalaee pushed a commit that referenced this pull request Feb 13, 2023
@Kludex @tiangolo
@adriangb @lovelydinosaur @aminalaee
Add WebSocketException and support for WS handlers (#1263)
299b5e4 
Co-authored-by: Sebastián Ramírez <tiangolo@gmail.com>
Co-authored-by: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com>
Co-authored-by: Tom Christie <tom@tomchristie.com>
@mnixry mnixry mentioned this pull request Aug 26, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers

@nameer nameer nameer left review comments

@lovelydinosaur lovelydinosaur lovelydinosaur approved these changes

@tiangolo tiangolo tiangolo approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@Kludex Kludex

Labels

feature New feature or request websocket WebSocket-related

Projects

None yet

Milestone

Version 0.21.0

Development

Successfully merging this pull request may close these issues.

7 participants

@Kludex @adriangb @nameer @brunopcarv @obonyojimmy @lovelydinosaur @tiangolo