-
Notifications
You must be signed in to change notification settings - Fork 511
feat: ✨ Native Websocket Plugin (uses okhttp) #1335
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
base: main
Are you sure you want to change the base?
feat: ✨ Native Websocket Plugin (uses okhttp) #1335
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Introduces a native WebSocket Cordova plugin for Android using OkHttp, providing a JavaScript WebSocket-like interface.
- Added a JS bridge (
www/websocket.js
) implementingconnect()
andWebSocketInstance
- Implemented Cordova plugin classes (
WebSocketPlugin.java
,WebSocketInstance.java
) using OkHttp for real WebSocket support - Updated
plugin.xml
, pluginpackage.json
, rootpackage.json
, andREADME.md
for installation and usage
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
File | Description |
---|---|
src/plugins/websocket/www/websocket.js | JS API: connect() returns WebSocketInstance and routes events |
src/plugins/websocket/src/android/WebSocketPlugin.java | CordovaPlugin routes actions (connect , send , close , listener) |
src/plugins/websocket/src/android/WebSocketInstance.java | OkHttp WebSocket listener with event marshaling to JS |
src/plugins/websocket/plugin.xml | Registers JS module and Android sources, declares OkHttp framework |
src/plugins/websocket/package.json | Plugin metadata, keywords, and license |
src/plugins/websocket/README.md | Usage guide and API reference |
package.json | Added plugin reference under cordovaPlugins and project deps |
Comments suppressed due to low confidence (1)
src/plugins/websocket/src/android/WebSocketInstance.java:15
- [nitpick] Core WebSocketInstance behavior (e.g., event callbacks and state transitions) lacks tests. Consider adding unit or integration tests to cover the listener methods.
public class WebSocketInstance extends WebSocketListener {
import java.util.HashMap; | ||
import java.util.UUID; | ||
|
||
// @TODO: plugin init & plugin destroy(closing okhttp clients) lifecycles. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The plugin init and destroy lifecycles are marked as TODO but not implemented. Implement proper initialization and cleanup (e.g., closing OkHttp clients) to prevent resource leaks.
Copilot uses AI. Check for mistakes.
this.onerror = null; | ||
this.url = url; | ||
|
||
exec((event) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Passing null as the error callback to exec may hide native errors. Consider providing an error handler to propagate failures back to JavaScript.
Copilot uses AI. Check for mistakes.
+1, copilot suggestions seem ok-ish. I'll add them by testing out. |
Co-authored-by: Copilot <[email protected]>
… functionality - Updated `WebSocketPlugin.connect` to accept custom headers. - Enhanced `WebSocketInstance.close` method to allow specifying close code and reason. - Added `WebSocketPlugin.listClients` method to list active WebSocket instances. - Updated JavaScript interface to support new features and improved documentation.
…ging and error handling - Updated `WebSocketInstance.send` and `close` methods to include detailed logging for better debugging. - Modified `WebSocketPlugin` to handle close operation errors and provide appropriate feedback to the callback context. - Ensured proper binding of event handlers in the JavaScript WebSocket interface for consistent context handling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds a native WebSocket Cordova plugin for Android using OkHttp, exposing a JS API that mimics the browser WebSocket interface and managing connections natively.
- Introduces a JS module (
websocket.js
) withconnect
,send
,close
, andlistClients
methods plus event handlers. - Implements
WebSocketPlugin.java
andWebSocketInstance.java
on Android, leveraging OkHttp for actual socket work. - Updates
plugin.xml
,package.json
, andREADME.md
for plugin registration, metadata, and documentation.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
src/plugins/websocket/www/websocket.js | JS wrapper providing promise-based connect, send, close, listClients |
src/plugins/websocket/src/android/WebSocketPlugin.java | CordovaPlugin execute logic, instance map, lifecycle management |
src/plugins/websocket/src/android/WebSocketInstance.java | OkHttp WebSocketListener handling events and sending to JS |
src/plugins/websocket/plugin.xml | Plugin manifest, Android feature and source-file declarations |
src/plugins/websocket/package.json | Plugin npm metadata and cordova integration settings |
src/plugins/websocket/README.md | Usage instructions and API reference |
package.json | Adds plugin to host app’s dependencies |
Comments suppressed due to low confidence (2)
src/plugins/websocket/www/websocket.js:68
- [nitpick] The exported 'send' and 'close' helper functions shadow the instance methods with the same names. Consider renaming them (e.g., 'sendById' / 'closeById') to improve clarity.
const send = function(instanceId, message) {
src/plugins/websocket/www/websocket.js:54
- The core API methods (connect, send, close, listClients) lack accompanying unit or integration tests. Consider adding tests to validate success and error flows.
const connect = function(url, protocols = null, headers = null) {
|
||
// TODO: plugin init & plugin destroy(closing okhttp clients) lifecycles. (✅) | ||
public class WebSocketPlugin extends CordovaPlugin { | ||
private static final HashMap<String, WebSocketInstance> instances = new HashMap<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The static HashMap 'instances' is not thread-safe and may lead to race conditions under concurrent access. Consider switching to a ConcurrentHashMap or synchronizing access.
Copilot uses AI. Check for mistakes.
<plugin id="cordova-plugin-websocket" version="0.0.1" xmlns="http://apache.org/cordova/ns/plugins/1.0"> | ||
<name>cordova-plugin-websocket</name> | ||
<description>Cordova Websocket</description> | ||
<license>MIT</license> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The license in plugin.xml is set to MIT, but package.json specifies Apache-2.0. Please align the license entries to avoid confusion for consumers.
Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <[email protected]>
Adds Native Websocket Cordova Plugin (using okhttp)
Important
plugin initialise(using okhttp singleton client) & destroy(closing okhttp builder clients) are to be completed. While the plugin is functional but those methods are important for plugin lifecycles. Also, custom headers support to be added.
Features
onopen
,onmessage
,onerror
,onclose
extensions
andreadyState
propertieslistClients()
to list active connectionslistClients()
to list active connectionsUsage
Usage
Import
Connect to WebSocket
API Reference
Methods
WebSocketPlugin.connect(url, protocols, headers)
url
: The WebSocket server URL.protocols
: (Optional) An array of subprotocol strings.headers
(object, optional): Custom headers as key-value pairs.WebSocketInstance
.WebSocketPlugin.listClients()
Promise
that resolves to an array ofinstanceId
strings.WebSocketPlugin.send(instanceId, message)
WebSocketInstance.send(message)
but needsinstanceId
.Promise
that resolves.WebSocketPlugin.close(instanceId, code, reason)
WebSocketInstance.close(code, reason)
but needsinstanceId
.Promise
that resolves.WebSocketInstance.send(message)
WebSocketInstance.close(code, reason)
code
: (Optional) If unspecified, a close code for the connection is automatically set: to 1000 for a normal closure, or otherwise to another standard value in the range 1001-1015 that indicates the actual reason the connection was closed.reason
: A string providing a custom WebSocket connection close reason (a concise human-readable prose explanation for the closure). The value must be no longer than 123 bytes (encoded in UTF-8).Properties of
WebSocketInstance
onopen
: Event listener for connection open.onmessage
: Event listener for messages received.onclose
: Event listener for connection close.onerror
: Event listener for errors.readyState
: (number) The state of the connection.CONNECTING
): Socket created, not yet open.OPEN
): Connection is open and ready.CLOSING
): Connection is closing.CLOSED
): Connection is closed or couldn't be opened.extensions
: (string) Extensions negotiated by the server (usually empty or a list).