Skip to content

Conversation

@jackzhp
Copy link

@jackzhp jackzhp commented Nov 8, 2025

null might be expected along the call stack, but here, null is not expected, so just return, and crash could be avoided.

when it crashes, req_wrap is null. For more information, please refer to #59465

…ted, but here, null is not expected, so just return, and crash could be avoided.

when it crashes, req_wrap is null.  For more information, please refer to nodejs#59465
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Nov 8, 2025
@aduh95
Copy link
Contributor

aduh95 commented Nov 8, 2025

Any chance you could add a test case that fails without this patch and passes with it?

@jackzhp
Copy link
Author

jackzhp commented Nov 8, 2025

Any chance you could add a test case that fails without this patch and passes with it?

Yes. I just post a comment for #60628. (client.ts & server.ts, with them you will see the client crashes the server very often). Please use them in any way as you wish. I don't know where to put these test files into this project.

By the way, #60628 was to merge it into v24.11.1, while this PR(#60629) was to merge it into main branch.

@aduh95
Copy link
Contributor

aduh95 commented Nov 8, 2025

By the way, #60628 was to merge it into v24.11.1, while this PR(#60629) was to merge it into main branch.

This first need to be merged to main, backported to v25.x and be released there for at least two weeks before it can land on v24.x. If every one was opening a PR for each release branch they wanted their changes to land, the repo would become unmanageable.

I don't know where to put these test files into this project.

You can have a look at doc/contributing/writing-tests.md for how to do it, the TL;DR is it should ideally be a single JS file inside test/parallel that exits with non-zero code with current main and with code 0 with your patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants