Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat(node/net): add SocketAddress #17154

Open
wants to merge 39 commits into
base: main
Choose a base branch
from
Open

feat(node/net): add SocketAddress #17154

wants to merge 39 commits into from

Conversation

DonIsaac
Copy link
Contributor

@DonIsaac DonIsaac commented Feb 7, 2025

What does this PR do?

Not ready for review. Creating a draft so tests can run.

  • Documentation or TypeScript types (it's okay to leave the rest blank in this case)
  • Code changes

TODO

  • swap out JSSocketAddress
  • lots of testing
  • Make sure it plays nicely with Listener

How did you verify your code works?

Lots of tests.

@DonIsaac DonIsaac added enhancement New feature or request node:net labels Feb 7, 2025
@robobun
Copy link

robobun commented Feb 7, 2025

Updated 2:48 PM PT - Feb 14th, 2025

@DonIsaac, your commit 5b80b73 has 4 failures in Build #11622:


🧪   try this PR locally:

bunx bun-pr 17154

errno: ?i32 = null,
name: ?string = null,
};
pub fn throwSysError(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

global.throwValue(bun.sys.Error.fromCode(errno, syscall).toJSC(global))

@DonIsaac
Copy link
Contributor Author

DonIsaac commented Feb 8, 2025

OFC Windows doesn't have socklen_t. Needs to be swapped out with c_ares.socklen_t

@DonIsaac DonIsaac changed the title [wip] feat(node/net): add SocketAddress feat(node/net): add SocketAddress Feb 11, 2025
@DonIsaac DonIsaac marked this pull request as ready for review February 14, 2025 01:49
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request node:net
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants