http: allow passing callbacks for [Incoming,Outgoing]Message#destroy

[rexagod]

Fixes: #28236

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• commit message follows commit guidelines

[lpinca]

I think we wanted to remove the callback from public destroy(), only the _destroy() implementation should take a callback.

cc: @nodejs/streams @ronag

[ronag]

I think we wanted to remove the callback from public destroy(), only the _destroy() implementation should take a callback.

Yup. This is an undocumented API we would like to remove.

You should do the following instead:

x.destroy(er)
finished(er, cb)

[mcollina]

lgtm

[ronag]

If we can backport this to 12 it would be good. It would fix #28236 which is already fixed on 14+ but that was through a semver-major change.

[lpinca]

We moved away from using the internal only, undocumented destroy(err, cb) api. This moves in the opposite direction and makes the api public.

[ronag]

We moved away from using the internal only, undocumented destroy(err, cb) api. This moves in the opposite direction and make the api public.

I agree, but it does fix a bug in v12 which would otherwise require a semver major change.

EDIT: I guess that could be fixed in a non semver-major way that does not require this PR... if a bit hackish.

[lpinca]

I would rather keep the bug than exposing an internal only, soon to be removed, api as public.

[github-actions]

This issue/PR was marked as stalled, it will be automatically closed in 30 days. If it should remain open, please leave a comment explaining why it should remain open.

[github-actions]

Closing this because it has stalled. Feel free to reopen if this PR is still relevant, or to ping the collaborator who labelled it stalled if you have any questions.