Skip to content

Implementing the TODO in pingora-core rustls listener to avoid panics by returning a Result#928

Open
omavashia2005 wants to merge 9 commits into
cloudflare:mainfrom
omavashia2005:fix/listener-unwrap-todo
Open

Implementing the TODO in pingora-core rustls listener to avoid panics by returning a Result#928
omavashia2005 wants to merge 9 commits into
cloudflare:mainfrom
omavashia2005:fix/listener-unwrap-todo

Conversation

@omavashia2005

@omavashia2005 omavashia2005 commented Jun 25, 2026

Copy link
Copy Markdown

Fixes #927

All tests pass with cargo test -p pingora-core

@omavashia2005 omavashia2005 changed the title Fix/listener unwrap todo Implementing the TODO in pingora-core rustls listener to avoid panics by returning a Result Jun 25, 2026
@omavashia2005 omavashia2005 marked this pull request as draft June 26, 2026 00:14
@omavashia2005 omavashia2005 marked this pull request as ready for review June 26, 2026 00:49

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Seems like this can still panic here vs. returning the Err?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Likewise, if we're going to change the API here anyhow, we should translate these panics into Errs as well.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Right, I was waiting for some feedback before changing this. I wanted to confirm whether those panics were intentional or if they should be changed as well before updating this and a few other panic!/unwrap() calls. Commits coming soon. Thanks for the feedback!

@omavashia2005

omavashia2005 commented Jun 26, 2026

Copy link
Copy Markdown
Author

Working on these still - mb for pushing - these currently fail with cargo build --features s2n and cargo build --features rustls

@omavashia2005

omavashia2005 commented Jun 26, 2026

Copy link
Copy Markdown
Author

this should be working now since cargo build --features s2n and cargo build --features rustls succeed.

@omavashia2005

Copy link
Copy Markdown
Author

Hey! Just following up...let me know if I need to make any further changes :)

Thanks!

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.

Finishing TODO in pingora-core rustls listeners to avoid panicking

2 participants