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

fix: handle proxy watcher errors #149

Conversation

george-e-shaw-iv
Copy link
Collaborator

  • i saw the todo so i tried my hand at handling proxy watcher errors. it
    made me realize that the underlying mcnet library doesn't respect
    context very well at all which is probably one of the most important
    places to respect context actually. we should consider contributing
    back to that library to upgrade net.Listen to use net.ListenConfig
    which does respect context.

Copy link
Member

@jaredallard jaredallard left a comment

Choose a reason for hiding this comment

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

Open question, but overall looks good. Appreciate it!

cmd/minecraft-preempt/proxy.go Outdated Show resolved Hide resolved
cmd/minecraft-preempt/proxy.go Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 0% with 31 lines in your changes are missing coverage. Please review.

Project coverage is 0.00%. Comparing base (e16a850) to head (6599a5d).

Files Patch % Lines
cmd/minecraft-preempt/proxy.go 0.00% 30 Missing ⚠️
cmd/minecraft-preempt/connection.go 0.00% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@          Coverage Diff          @@
##            main    #149   +/-   ##
=====================================
  Coverage   0.00%   0.00%           
=====================================
  Files         11      11           
  Lines        578     596   +18     
=====================================
- Misses       578     596   +18     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

codecov bot commented May 18, 2024

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

@jaredallard
Copy link
Member

@george-e-shaw-iv Do you mind signing your commits please? 🙇

@george-e-shaw-iv george-e-shaw-iv force-pushed the george-e-shaw-iv/fix/handle-proxy-errors branch from 64e8575 to 778986f Compare May 18, 2024 21:54
George Shaw added 2 commits May 18, 2024 15:07
* i saw the todo so i tried my hand at handling proxy watcher errors. it
  made me realize that the underlying mcnet library doesn't respect
  context very well at all which is probably one of the most important
  places to respect context actually. we should consider contributing
  back to that library to upgrade net.Listen to use net.ListenConfig
  which does respect context.
@george-e-shaw-iv george-e-shaw-iv force-pushed the george-e-shaw-iv/fix/handle-proxy-errors branch from 778986f to bd92681 Compare May 18, 2024 22:08
@jaredallard jaredallard merged commit 273839b into rgst-io:main May 18, 2024
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.

3 participants