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

EventedStreamReader not really emitting OnCompleteLine() per each line received #6146

Closed
dgaspar opened this issue Dec 27, 2018 · 2 comments
Closed
Assignees
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates bug This issue describes a behavior which is not expected - a bug. Done This issue has been fixed help wanted Up for grabs. We would accept a PR to help resolve this issue
Milestone

Comments

@dgaspar
Copy link
Contributor

dgaspar commented Dec 27, 2018

Describe the bug

EventedStreamReader's Run() method sometimes reads chunks containing multiple \n, but it only cares about the first \n it sees, and buffers the rest. The problem is when the chunk is the last output coming of npm during npm run serve, for a while. The remaining buffer is not notified (OnCompleteLine() is not called).

The risk is WaitFor(Regex regex) randomly missing the opportunity to finish, because it might miss some lines, as it listens to onReceivedLineHandler, and not to chunk events.

To Reproduce

Steps to reproduce the behavior:

  1. ASP.Net Core 2.2 with Spa extensions;
  2. tricky to reproduce: the app reads from npm's stdout a stream chunk that contains multiple \n characters in it;
  3. notice how only the first chunk (until the first \n) gets notified to the event listeners, the rest doesn't.

Expected behavior

I would expect one call to OnCompleteLine() per actual line.

EEParker added a commit to EEParker/aspnetcore-vueclimiddleware that referenced this issue Jan 8, 2019
@Eilon Eilon added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Jan 18, 2019
@mkArtakMSFT mkArtakMSFT added help wanted Up for grabs. We would accept a PR to help resolve this issue PRI: 2 - Preferred bug This issue describes a behavior which is not expected - a bug. labels Jul 3, 2019
@mkArtakMSFT mkArtakMSFT added this to the Backlog milestone Jul 3, 2019
@mkArtakMSFT
Copy link
Member

Thanks for contacting us, @dgaspar.
Would you be interested in sending us a PR for this? We'll happily consider it.

ryanbrandenburg added a commit that referenced this issue Aug 28, 2019
Handle chunks containing multiple EOL (#6146)
@mkArtakMSFT mkArtakMSFT added the Done This issue has been fixed label Aug 29, 2019
@mkArtakMSFT
Copy link
Member

Thanks for your contribution, @dgaspar👍

@ghost ghost locked as resolved and limited conversation to collaborators Dec 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates bug This issue describes a behavior which is not expected - a bug. Done This issue has been fixed help wanted Up for grabs. We would accept a PR to help resolve this issue
Projects
None yet
Development

No branches or pull requests

4 participants