blinkdog/telnet-stream

protocol errors should emit an error event

Closed this issue · 1 comments

Right now the code silently eats protocol errors, attempting to ignore them.

This is bad for several reasons.

  • Silently discarding errors may hide the fact that any errors occurred at all
  • Downstream code is not allowed to make the stop/continue decision

I think I was in a fault-tolerance mindset at the time.
I was probably wondering, "What should I do if the protocol goes into an undefined state?"
I'm not sure why the answer wasn't painfully obvious at the time.
The answer is in the Node.js stream model: Emit an error event.

This would stop the silent discard AND give downstream code a chance to handle as it sees fit.

telnet-stream v1.0.3 now emits error events for subnegotiation errors:

  • subnegotiation buffer overflows
  • subnegotiation command errors

The optional options object also allows the user to configure the behavior (keep the bytes, discard the bytes) in case of command errors.

The default behavior was changed from a silent discard to keeping both bytes.