Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

chore: add network extension manager#18

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
ethanndickson merged 5 commits intomainfromethan/ne-manager
Jan 14, 2025
Merged

Conversation

ethanndickson
Copy link
Member

@ethanndicksonethanndickson commentedJan 8, 2025
edited
Loading

Relates to#2.

Adds theManager abstraction that:

  • Downloads the dylib
  • Calls theSignatureValidator on a downloaded dylib
  • Passes network settings toNEPacketTunnelProvider
  • Owns theTunnelHandle
  • Reads and writes to theSpeaker

Eventually, it'll act as the middleman between the tunnel Speaker and the XPC speaker.

@ethanndicksonGraphite App
Copy link
MemberAuthor

ethanndickson commentedJan 8, 2025
edited
Loading

@ethanndicksonethanndickson marked this pull request as ready for reviewJanuary 8, 2025 09:57
@ethanndicksonethanndickson marked this pull request as draftJanuary 8, 2025 09:58
@ethanndicksonethanndicksonforce-pushed theethan/ne-manager branch 6 times, most recently from617a941 to300889eCompareJanuary 10, 2025 04:07
@@ -4,16 +4,200 @@ import VPNLib

actor Manager {
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

I don't think this is worth unit testing, at least right now. We'd need to mock the PTP, the TunnelHandle, the validator, and eventually the XPC speaker, all for a relatively simple abstraction that's mostly error handling.

Comment on lines +215 to +216
// This can't be checked at compile-time, as both Tasks & Continuations can only ever throw
// a type-erased `Error`
Copy link
MemberAuthor

@ethanndicksonethanndicksonJan 10, 2025
edited
Loading

Choose a reason for hiding this comment

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

This is a little frustrating - theTask type itself is generic on two type parameters,Success andFailure, but the only APIs you can await on a task are eithervalue with an untypedthrows orresult which returns a type-erasedany Error error variant. It's very similar for continuations, the type itself is generic but there's no way to return that typed error.

As an aside, I think I'm becoming more of a fan, for binaries at least, of just having a single opaque error type, propagating it with context, and then just displaying it, ala Go or Rust'sanyhow - it's so rare that one or more of the error variants are recoverable, and that the caller needs to know which one.

@ethanndicksonethanndicksonforce-pushed theethan/ne-manager branch 2 times, most recently from4922fcf to7e24349CompareJanuary 10, 2025 05:28
@ethanndicksonethanndickson marked this pull request as ready for reviewJanuary 10, 2025 05:38
@ethanndicksonethanndickson self-assigned thisJan 10, 2025
@ethanndickson
Copy link
MemberAuthor

@coadler Requesting your review here since Spike is OOO, and this is pretty relevant to you. This is the portion of the network extension that communicates over XPC.

@ethanndicksonethanndickson changed the base branch fromethan/dylib-download tographite-base/18January 14, 2025 04:48
@ethanndicksonethanndickson changed the base branch fromgraphite-base/18 tomainJanuary 14, 2025 04:49
@ethanndicksonGraphite App
Copy link
MemberAuthor

ethanndickson commentedJan 14, 2025
edited
Loading

Merge activity

  • Jan 14, 12:19 AM EST: A user started a stack merge that includes this pull request viaGraphite.
  • Jan 14, 12:19 AM EST: A user merged this pull request withGraphite.

@ethanndicksonethanndickson merged commit46c2c09 intomainJan 14, 2025
4 checks passed
@ethanndicksonethanndickson deleted the ethan/ne-manager branchJanuary 14, 2025 05:22
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@coadlercoadlercoadler approved these changes

@spikecurtisspikecurtisAwaiting requested review from spikecurtis

Assignees

@ethanndicksonethanndickson

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@ethanndickson@coadler

[8]ページ先頭

©2009-2025 Movatter.jp