search

spdy-http2 / node-spdy

SPDY server on Node.js
2.81k stars 196 forks source link

fix [DEP0066] warning for node > 10.x #367

Open FlorentinTh opened 4 years ago

FlorentinTh commented 4 years ago

Just a simple change from : this._headers to this.getHeaders() in line 18 of response.js. It should be enough to remove the warning [DEP0066] DeprecationWarning: OutgoingMessage.prototype._headers is deprecated for users of Node > 10.x

masx200 commented 4 years ago

Support it. Great. Well done.

FlorentinTh commented 4 years ago

Yeah but it needs improvement to support node < 7.x I found the solution!

FlorentinTh commented 4 years ago

I added a ternary variable assignment to check either the new response.getHeaders() is available (for node >= 7.x) or not (node < 7.x). In the last case, it still using the old response.headers

masx200 commented 4 years ago

@indutny @anandsuresh @eee-c @anandsuresh @daviddias

LuigiMaestrelli commented 3 years ago

Any news on this issue? Thanks

FlorentinTh commented 3 years ago

Nope. No one seems to be available to help on tests with node > 10 that I don’t have time to go deeper in to understand. Moreover they have to allow pull request also with no one responding...

franz-josef-kaiser commented 3 years ago

@daviddias Can you please take a look and invest 5 mins of your time here? It's an easy fix and people really need it to move forward. Thanks a lot, it will be highly appreciated!

daviddias commented 3 years ago

Thanks for the nudge @franz-josef-kaiser. I'm not actively maintaining this package for a very long time, having passed that baton to @jacobheun. Let me ping him so that he can review it better :)

attila-boros commented 2 years ago

Hi @FlorentinTh, thanks for the fix!

I ran test/client-test.js separately with

npm exec mocha test/client-test.js

on Node v14.17.4 (LTS) and noticed that POST requests and their "after each" hooks are failing in SSL mode. If I move the it('should send GET request' test after it('should send POST request', the tests are passing. So GET after POST is working, but the other way it isn't. In general the order in which the tests are executed should not matter, probably some resource is not cleaned up properly.

I hope the maintainers can take a look at this.

FlorentinTh commented 2 years ago

Nice work @attila-boros. At least you figured out the issue with failing tests giving less work to the maintainers. I hope so the PR will be merged and released one day.

lamweili commented 2 years ago

@indutny Similar to https://github.com/spdy-http2/http-deceiver/pull/7#issuecomment-947454875, can you review this as well?

/cc @alexander-akait

ZLester commented 2 years ago

@jacobheun @daviddias Any news on this one?

lamweili commented 2 years ago

cc: @jacobheun @daviddias @indutny

nickscialli-msft commented 1 year ago

Wanted to bump this as well as I'm getting warnings on my http2/express server using the node-spdy package!

Also, does anyone on this thread know the potential effects of this deprecation? Can it affect/drop headers or is more along the lines of using an existing (but unsupported) API? Thanks!

beenotung commented 1 year ago

@indutny would you take a few minutes to review this PR or grant some of us the permission to accept PR (and publish on npm)?

Otherwise I'm willing to take the load to fork this package and get this PR published to npm. Appreciate the contribution from everyone 🙏

beenotung commented 1 year ago

I've forked this package into spdy-fixes to get the fixes published to npm. I'm happy to merge the fixes upstream if the author return to this project.

I may try to look into other issues and pull request upon suggestions. Appreciate the contributions from everyone.