Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deprecate IncomingMessage 'aborted' event #15456

Closed
ronag opened this issue Sep 18, 2017 · 8 comments
Closed

deprecate IncomingMessage 'aborted' event #15456

ronag opened this issue Sep 18, 2017 · 8 comments
Labels
http Issues or PRs related to the http subsystem.

Comments

@ronag
Copy link
Member

ronag commented Sep 18, 2017

The aborted event on IncomingMessage seems to do the exact same thing as the close event. I suggest that aborted is deprecated for reduced complexity and ambiguousness.

@ronag ronag closed this as completed Sep 18, 2017
@ronag ronag reopened this Sep 18, 2017
@jasnell
Copy link
Member

jasnell commented Sep 18, 2017

ping @nodejs/http

@jasnell jasnell added the http Issues or PRs related to the http subsystem. label Sep 18, 2017
@ronag
Copy link
Member Author

ronag commented Sep 18, 2017

At this point I think it would be enough to just remove it from the documentation but leave it in the code for backwards compat.

@lpinca
Copy link
Member

lpinca commented Sep 19, 2017

I agree, it seems redundant but I think it doesn't cost us anything to maintain 1 line of code and 1 line of documentation.

@ronag
Copy link
Member Author

ronag commented Sep 19, 2017

@lpinca: Other than users trying to figure out when to use what when reading the docs.

@lpinca
Copy link
Member

lpinca commented Sep 19, 2017

@ronag is it confusing for you? If so can you please explain why?
I find it normal to be emitted before 'close' but we can make it explicit in the docs.

@ronag
Copy link
Member Author

ronag commented Sep 19, 2017

@lpinca: It adds a layer of complexity. It's already confusing in general when and what events will be emitted and in what order. Currently, I and my teammates have to read the node code for every event to try and understand how they work. The fewer events there are the easier it is.

Having two events doing the same thing is weird and I've been trying to figure out the difference... which I assumed existed but it turns out it wasn't there.

@ronag
Copy link
Member Author

ronag commented Sep 19, 2017

I've been trying to put a little effort into figuring out and making the whole event behavior easier:

#15469
#15456
#15387
#15361
#15360
#15318
#15317
#15303
#15259

@apapirovski
Copy link
Member

There has been no movement and we're unlikely to get rid of it as things stand. I'm going to close this for now but maybe we'll revisit at some point in the future if http gets a bit more attention.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
http Issues or PRs related to the http subsystem.
Projects
None yet
Development

No branches or pull requests

4 participants