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

socket: use socket.destroy() properly #84

Closed
wants to merge 1 commit into from
Closed

Conversation

aqrln
Copy link
Member

@aqrln aqrln commented Feb 22, 2017

socket.destroy() should only be called in case of application errors
that make it impossible to continue working with this socket (like
protocol parsing errors, exactly our situation). This method emits
error event on socket automatically if an exception is passed as the
first argument.

`socket.destroy()` should only be called in case of application errors
that make it impossible to continue working with this socket (like
protocol parsing errors, exactly our situation). This method emits
`error` event on socket automatically if an exception is passed as the
first argument.
@aqrln
Copy link
Member Author

aqrln commented Feb 22, 2017

Maybe we should revert fc1d99d after this.

@belochub
Copy link
Member

@aqrln, what about semver for this PR? Can you add a corresponding label?

@aqrln
Copy link
Member Author

aqrln commented Feb 26, 2017

@belochub not that it should break anything, I highly doubt that, but I feel like it's worth landing on 0.7 and leaving 0.6 as it is.

@aqrln
Copy link
Member Author

aqrln commented Mar 1, 2017

This will be superseded by #92. When rebasing 1982152, a Fixes: field should be added if this PR won't have been landed by that time.

@aqrln
Copy link
Member Author

aqrln commented Mar 27, 2017

@belochub ping. Are there any objections to this change or I can get a sign-off and land it?

@belochub
Copy link
Member

@aqrln, I have no objections to this change, you can land it.

@aqrln
Copy link
Member Author

aqrln commented Mar 27, 2017

Landed in 0fd5c97.

aqrln added a commit that referenced this pull request Mar 27, 2017
`socket.destroy()` should only be called in case of application errors
that make it impossible to continue working with this socket (like
protocol parsing errors, exactly our situation). This method emits
`error` event on socket automatically if an exception is passed as the
first argument.

PR-URL: #84
@aqrln aqrln closed this Mar 27, 2017
@aqrln aqrln deleted the proper-socket-destroy branch March 27, 2017 16:55
aqrln added a commit that referenced this pull request Mar 27, 2017
`socket.destroy()` should only be called in case of application errors
that make it impossible to continue working with this socket (like
protocol parsing errors, exactly our situation). This method emits
`error` event on socket automatically if an exception is passed as the
first argument.

PR-URL: #84
aqrln added a commit that referenced this pull request Mar 27, 2017
`socket.destroy()` should only be called in case of application errors
that make it impossible to continue working with this socket (like
protocol parsing errors, exactly our situation). This method emits
`error` event on socket automatically if an exception is passed as the
first argument.

PR-URL: #84
Reviewed-by: Mykola Bilochub <nbelochub@gmail.com>
aqrln added a commit that referenced this pull request Apr 2, 2017
`socket.destroy()` should only be called in case of application errors
that make it impossible to continue working with this socket (like
protocol parsing errors, exactly our situation). This method emits
`error` event on socket automatically if an exception is passed as the
first argument.

Backport-of: #84
aqrln added a commit that referenced this pull request Apr 2, 2017
`socket.destroy()` should only be called in case of application errors
that make it impossible to continue working with this socket (like
protocol parsing errors, exactly our situation). This method emits
`error` event on socket automatically if an exception is passed as the
first argument.

Backport-of: #84
belochub pushed a commit that referenced this pull request Jan 22, 2018
`socket.destroy()` should only be called in case of application errors
that make it impossible to continue working with this socket (like
protocol parsing errors, exactly our situation). This method emits
`error` event on socket automatically if an exception is passed as the
first argument.

PR-URL: #84
Reviewed-by: Mykola Bilochub <nbelochub@gmail.com>
belochub pushed a commit that referenced this pull request Jan 22, 2018
`socket.destroy()` should only be called in case of application errors
that make it impossible to continue working with this socket (like
protocol parsing errors, exactly our situation). This method emits
`error` event on socket automatically if an exception is passed as the
first argument.

PR-URL: #84
Reviewed-by: Mykola Bilochub <nbelochub@gmail.com>
belochub added a commit that referenced this pull request Jan 22, 2018
This is a new and shiny first major release for `metarhia-jstp`.
Changes include API refactoring and improvements, implementations of
CLI, sessions, and application versions, native addon build optimizations,
lots of bug fixes, test coverage increase, and other, less notable changes.

This release also denotes the bump of the protocol version to v1.0.
The only difference from the previous version of the protocol is that
"old" heartbeat messages (`{}`) are now deprecated and `ping`/`pong`
messages must be used for this purpose instead.

Notable changes:

 * **src,build:** improve the native module subsystem
   *(Alexey Orlenko)*
   [#36](#36)
   **\[semver-minor\]**
 * **build:** compile in ISO C++11 mode
   *(Alexey Orlenko)*
   [#37](#37)
   **\[semver-minor\]**
 * **build:** improve error handling
   *(Alexey Orlenko)*
   [#40](#40)
   **\[semver-minor\]**
 * **lib:** refactor record-serialization.js
   *(Alexey Orlenko)*
   [#41](#41)
   **\[semver-minor\]**
 * **protocol:** change the format of handshake packets
   *(Alexey Orlenko)*
   [#54](#54)
   **\[semver-major\]**
 * **parser:** remove special case for '\0' literal
   *(Mykola Bilochub)*
   [#68](#68)
   **\[semver-major\]**
 * **client:** drop redundant callback argument
   *(Alexey Orlenko)*
   [#104](#104)
   **\[semver-major\]**
 * **client:** handle errors in connectAndInspect
   *(Alexey Orlenko)*
   [#105](#105)
   **\[semver-major\]**
 * **socket,ws:** use socket.destroy() properly
   *(Alexey Orlenko)*
   [#84](#84)
   **\[semver-major\]**
 * **cli:** add basic implementation
   *(Mykola Bilochub)*
   [#107](#107)
   **\[semver-minor\]**
 * **connection:** fix error handling in optional cbs
   *(Alexey Orlenko)*
   [#147](#147)
   **\[semver-major\]**
 * **lib:** change event signature
   *(Denys Otrishko)*
   [#187](#187)
   **\[semver-major\]**
 * **lib:** add address method to Server
   *(Denys Otrishko)*
   [#190](#190)
   **\[semver-minor\]**
 * **lib:** optimize connection events
   *(Denys Otrishko)*
   [#222](#222)
   **\[semver-major\]**
 * **lib:** refactor server and client API
   *(Denys Otrishko)*
   [#209](#209)
   **\[semver-major\]**
 * **lib,src:** rename term packet usages to message
   *(Denys Otrishko)*
   [#270](#270)
   **\[semver-major\]**
 * **lib:** emit events about connection messages
   *(Denys Otrishko)*
   [#252](#252)
   **\[semver-minor\]**
 * **connection:** make callback method private
   *(Alexey Orlenko)*
   [#306](#306)
   **\[semver-major\]**
 * **lib:** implement sessions
   *(Mykola Bilochub)*
   [#289](#289)
   **\[semver-major\]**
 * **connection:** use ping-pong instead of heartbeat
   *(Dmytro Nechai)*
   [#303](#303)
   **\[semver-major\]**
@belochub belochub mentioned this pull request Jan 22, 2018
belochub added a commit that referenced this pull request Jan 23, 2018
This is a new and shiny first major release for `metarhia-jstp`.
Changes include API refactoring and improvements, implementations of
CLI, sessions, and application versions, native addon build optimizations,
lots of bug fixes, test coverage increase, and other, less notable changes.

This release also denotes the bump of the protocol version to v1.0.
The only difference from the previous version of the protocol is that
"old" heartbeat messages (`{}`) are now deprecated and `ping`/`pong`
messages must be used for this purpose instead.

Notable changes:

 * **src,build:** improve the native module subsystem
   *(Alexey Orlenko)*
   [#36](#36)
   **\[semver-minor\]**
 * **build:** compile in ISO C++11 mode
   *(Alexey Orlenko)*
   [#37](#37)
   **\[semver-minor\]**
 * **build:** improve error handling
   *(Alexey Orlenko)*
   [#40](#40)
   **\[semver-minor\]**
 * **lib:** refactor record-serialization.js
   *(Alexey Orlenko)*
   [#41](#41)
 * **parser:** fix a possible memory leak
   *(Alexey Orlenko)*
   [#44](#44)
   **\[semver-minor\]**
 * **protocol:** change the format of handshake packets
   *(Alexey Orlenko)*
   [#54](#54)
   **\[semver-major\]**
 * **parser:** make parser single-pass
   *(Mykola Bilochub)*
   [#61](#61)
 * **parser:** remove special case for '\0' literal
   *(Mykola Bilochub)*
   [#68](#68)
   **\[semver-major\]**
 * **parser:** fix bug causing node to crash
   *(Mykola Bilochub)*
   [#75](#75)
 * **client:** drop redundant callback argument
   *(Alexey Orlenko)*
   [#104](#104)
   **\[semver-major\]**
 * **client:** handle errors in connectAndInspect
   *(Alexey Orlenko)*
   [#105](#105)
   **\[semver-major\]**
 * **socket,ws:** use socket.destroy() properly
   *(Alexey Orlenko)*
   [#84](#84)
   **\[semver-major\]**
 * **cli:** add basic implementation
   *(Mykola Bilochub)*
   [#107](#107)
   **\[semver-minor\]**
 * **connection:** fix error handling in optional cbs
   *(Alexey Orlenko)*
   [#147](#147)
   **\[semver-major\]**
 * **test:** add JSON5 specs test suite
   *(Alexey Orlenko)*
   [#158](#158)
 * **lib:** change event signature
   *(Denys Otrishko)*
   [#187](#187)
   **\[semver-major\]**
 * **lib:** add address method to Server
   *(Denys Otrishko)*
   [#190](#190)
   **\[semver-minor\]**
 * **parser:** implement NaN and Infinity parsing
   *(Mykola Bilochub)*
   [#201](#201)
 * **parser:** improve string parsing performance
   *(Mykola Bilochub)*
   [#220](#220)
 * **lib:** optimize connection events
   *(Denys Otrishko)*
   [#222](#222)
   **\[semver-major\]**
 * **lib:** refactor server and client API
   *(Denys Otrishko)*
   [#209](#209)
   **\[semver-major\]**
 * **lib,src:** rename term packet usages to message
   *(Denys Otrishko)*
   [#270](#270)
   **\[semver-major\]**
 * **lib:** emit events about connection messages
   *(Denys Otrishko)*
   [#252](#252)
   **\[semver-minor\]**
 * **lib:** implement API versioning
   *(Denys Otrishko)*
   [#231](#231)
   **\[semver-minor\]**
 * **lib:** allow to set event handlers in application
   *(Denys Otrishko)*
   [#286](#286)
   **\[semver-minor\]**
 * **lib:** allow to broadcast events from server
   *(Denys Otrishko)*
   [#287](#287)
   **\[semver-minor\]**
 * **connection:** make callback method private
   *(Alexey Orlenko)*
   [#306](#306)
   **\[semver-major\]**
 * **lib:** implement sessions
   *(Mykola Bilochub)*
   [#289](#289)
   **\[semver-major\]**
 * **connection:** use ping-pong instead of heartbeat
   *(Dmytro Nechai)*
   [#303](#303)
   **\[semver-major\]**
belochub pushed a commit to metarhia/mdsf that referenced this pull request Jul 19, 2018
`socket.destroy()` should only be called in case of application errors
that make it impossible to continue working with this socket (like
protocol parsing errors, exactly our situation). This method emits
`error` event on socket automatically if an exception is passed as the
first argument.

PR-URL: metarhia/jstp#84
Reviewed-by: Mykola Bilochub <nbelochub@gmail.com>
belochub pushed a commit to metarhia/mdsf that referenced this pull request Jul 19, 2018
`socket.destroy()` should only be called in case of application errors
that make it impossible to continue working with this socket (like
protocol parsing errors, exactly our situation). This method emits
`error` event on socket automatically if an exception is passed as the
first argument.

PR-URL: metarhia/jstp#84
Reviewed-by: Mykola Bilochub <nbelochub@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants