-
Notifications
You must be signed in to change notification settings - Fork 38
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
MIDParser silently errors because _destroy() is a no-op #33
Comments
Additionally, I think that SessionControlClient should be modified to emit an error when calling close() with an error. I tried removing the _destroy() noop mentioned above, but still didn't get an error. Here's a sample output with `NODE_DEBUG=open-protocol`, with the changes mentioned
No error is ever emitted by the SessionControlClient. Adding an |
ferm10n
pushed a commit
to ferm10n/node-open-protocol
that referenced
this issue
Jun 2, 2022
ferm10n
pushed a commit
to RV-Argonaut/node-open-protocol
that referenced
this issue
Jun 2, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I'm discovering that if there's an error during parsing, MidParser does not emit it, and the stream is silently destroyed. This might be caused by something I did while tinkering with the project, but it'd really help if I had some context about this section:
node-open-protocol/src/MIDParser.js
Lines 71 to 73 in f514d00
It seems to be responsible for why the error gets eaten. If I remove it, the default handler emits the error event correctly.
How did earlier versions of node behave that prompted _destroy() to become a noop? I'd like to fix this without breaking backwards compat, but can't do that without more history on why it was added.
(I'm using node v14 btw)
The text was updated successfully, but these errors were encountered: