Skip to content

Remove use of Module['websocket']. NFC #22774

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Nov 1, 2024
Merged

Remove use of Module['websocket']. NFC #22774

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 50 additions & 62 deletions src/library_sockfs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,37 +10,35 @@ addToLibrary({
},
$SOCKFS__deps: ['$FS'],
$SOCKFS: {
#if expectToReceiveOnModule('websocket')
websocketArgs: {},
#endif
callbacks: {},
on(event, callback) {
SOCKFS.callbacks[event] = callback;
},
emit(event, param) {
SOCKFS.callbacks[event]?.(param);
},
mount(mount) {
// If Module['websocket'] has already been defined (e.g. for configuring
// the subprotocol/url) use that, if not initialise it to a new object.
Module['websocket'] = (Module['websocket'] &&
('object' === typeof Module['websocket'])) ? Module['websocket'] : {};

#if expectToReceiveOnModule('websocket')
// The incomming Module['websocket'] can be used for configuring
// configuring subprotocol/url, etc
SOCKFS.websocketArgs = {{{ makeModuleReceiveExpr('websocket', '{}') }}};
// Add the Event registration mechanism to the exported websocket configuration
// object so we can register network callbacks from native JavaScript too.
// For more documentation see system/include/emscripten/emscripten.h
Module['websocket']._callbacks = {};
Module['websocket']['on'] = /** @this{Object} */ function(event, callback) {
if ('function' === typeof callback) {
this._callbacks[event] = callback;
}
return this;
};

Module['websocket'].emit = /** @this{Object} */ function(event, param) {
if ('function' === typeof this._callbacks[event]) {
this._callbacks[event].call(this, param);
}
};
(Module['websocket'] ??= {})['on'] = SOCKFS.on;
Copy link
Member

@kripken kripken Oct 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we not need 'emit' as well? Looks like before we had both exported.

Copy link
Collaborator Author

@sbc100 sbc100 Oct 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I don't think so. The emit for for internal use to send the message. The on is what users would use to register callback.

Copy link
Collaborator Author

@sbc100 sbc100 Oct 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can see that the on property is quoted so that closure won't minify it but the emit helper function is not.

Copy link
Collaborator Author

@sbc100 sbc100 Oct 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the docs here: https://emscripten.org/docs/api_reference/emscripten.h.html#socket-event-registration

#endif

// If debug is enabled register simple default logging callbacks for each Event.
#if SOCKET_DEBUG
Module['websocket']['on']('error', (error) => dbg('websocket: error ' + error));
Module['websocket']['on']('open', (fd) => dbg('websocket: open fd = ' + fd));
Module['websocket']['on']('listen', (fd) => dbg('websocket: listen fd = ' + fd));
Module['websocket']['on']('connection', (fd) => dbg('websocket: connection fd = ' + fd));
Module['websocket']['on']('message', (fd) => dbg('websocket: message fd = ' + fd));
Module['websocket']['on']('close', (fd) => dbg('websocket: close fd = ' + fd));
// If debug is enabled register simple default logging callbacks for each Event.
SOCKFS.on('error', (error) => dbg('websocket: error ' + error));
SOCKFS.on('open', (fd) => dbg('websocket: open fd = ' + fd));
SOCKFS.on('listen', (fd) => dbg('websocket: listen fd = ' + fd));
SOCKFS.on('connection', (fd) => dbg('websocket: connection fd = ' + fd));
SOCKFS.on('message', (fd) => dbg('websocket: message fd = ' + fd));
SOCKFS.on('close', (fd) => dbg('websocket: close fd = ' + fd));
#endif

return FS.createNode(null, '/', {{{ cDefs.S_IFDIR }}} | 511 /* 0777 */, 0);
Expand Down Expand Up @@ -169,36 +167,32 @@ addToLibrary({
} else {
// create the actual websocket object and connect
try {
// runtimeConfig gets set to true if WebSocket runtime configuration is available.
var runtimeConfig = (Module['websocket'] && ('object' === typeof Module['websocket']));

// The default value is 'ws://' the replace is needed because the compiler replaces '//' comments with '#'
// comments without checking context, so we'd end up with ws:#, the replace swaps the '#' for '//' again.
var url = '{{{ WEBSOCKET_URL }}}'.replace('#', '//');
// Make the WebSocket subprotocol (Sec-WebSocket-Protocol) default to binary if no configuration is set.
var subProtocols = '{{{ WEBSOCKET_SUBPROTOCOL }}}'; // The default value is 'binary'
// The default WebSocket options
var opts = undefined;

if (runtimeConfig) {
if ('string' === typeof Module['websocket']['url']) {
url = Module['websocket']['url']; // Fetch runtime WebSocket URL config.
}
#if expectToReceiveOnModule('websocket')
// Fetch runtime WebSocket URL config.
if (SOCKFS.websocketArgs['url']) {
url = SOCKFS.websocketArgs['url'];
}
// Fetch runtime WebSocket subprotocol config.
if (SOCKFS.websocketArgs['subprotocol']) {
subProtocols = SOCKFS.websocketArgs['subprotocol'];
} else if (SOCKFS.websocketArgs['subprotocol'] === null) {
subProtocols = 'null'
Copy link
Member

@kripken kripken Oct 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
subProtocols = 'null'
subProtocols = 'null';

}
#endif

if (url === 'ws://' || url === 'wss://') { // Is the supplied URL config just a prefix, if so complete it.
var parts = addr.split('/');
url = url + parts[0] + ":" + port + "/" + parts.slice(1).join('/');
}

// Make the WebSocket subprotocol (Sec-WebSocket-Protocol) default to binary if no configuration is set.
var subProtocols = '{{{ WEBSOCKET_SUBPROTOCOL }}}'; // The default value is 'binary'

if (runtimeConfig) {
if ('string' === typeof Module['websocket']['subprotocol']) {
subProtocols = Module['websocket']['subprotocol']; // Fetch runtime WebSocket subprotocol config.
}
}

// The default WebSocket options
var opts = undefined;

if (subProtocols !== 'null') {
// The regex trims the string (removes spaces at the beginning and end, then splits the string by
// <any space>,<any space> into an Array. Whitespace removal is important for Websockify and ws.
Expand All @@ -207,12 +201,6 @@ addToLibrary({
opts = subProtocols;
}

// some webservers (azure) does not support subprotocol header
if (runtimeConfig && null === Module['websocket']['subprotocol']) {
subProtocols = 'null';
opts = undefined;
}
Copy link
Member

@kripken kripken Oct 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Separate change? (I don't seem to see this code moved anywhere else.)

Copy link
Collaborator Author

@sbc100 sbc100 Oct 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I specially handle the === null case above.


#if SOCKET_DEBUG
dbg('websocket: connect: ' + url + ', ' + subProtocols.toString());
#endif
Expand Down Expand Up @@ -280,8 +268,8 @@ addToLibrary({
dbg('websocket: handle open');
#endif

Module['websocket'].emit('open', sock.stream.fd);
sock.connecting = false;
SOCKFS.emit('open', sock.stream.fd);

try {
var queued = peer.msg_send_queue.shift();
Expand Down Expand Up @@ -334,7 +322,7 @@ addToLibrary({
}

sock.recv_queue.push({ addr: peer.addr, port: peer.port, data: data });
Module['websocket'].emit('message', sock.stream.fd);
SOCKFS.emit('message', sock.stream.fd);
};

if (ENVIRONMENT_IS_NODE) {
Expand All @@ -346,21 +334,21 @@ addToLibrary({
handleMessage((new Uint8Array(data)).buffer); // copy from node Buffer -> ArrayBuffer
});
peer.socket.on('close', function() {
Module['websocket'].emit('close', sock.stream.fd);
SOCKFS.emit('close', sock.stream.fd);
});
peer.socket.on('error', function(error) {
// Although the ws library may pass errors that may be more descriptive than
// ECONNREFUSED they are not necessarily the expected error code e.g.
// ENOTFOUND on getaddrinfo seems to be node.js specific, so using ECONNREFUSED
// is still probably the most useful thing to do.
sock.error = {{{ cDefs.ECONNREFUSED }}}; // Used in getsockopt for SOL_SOCKET/SO_ERROR test.
Module['websocket'].emit('error', [sock.stream.fd, sock.error, 'ECONNREFUSED: Connection refused']);
SOCKFS.emit('error', [sock.stream.fd, sock.error, 'ECONNREFUSED: Connection refused']);
// don't throw
});
} else {
peer.socket.onopen = handleOpen;
peer.socket.onclose = function() {
Module['websocket'].emit('close', sock.stream.fd);
SOCKFS.emit('close', sock.stream.fd);
};
peer.socket.onmessage = function peer_socket_onmessage(event) {
handleMessage(event.data);
Expand All @@ -369,7 +357,7 @@ addToLibrary({
// The WebSocket spec only allows a 'simple event' to be thrown on error,
// so we only really know as much as ECONNREFUSED.
sock.error = {{{ cDefs.ECONNREFUSED }}}; // Used in getsockopt for SOL_SOCKET/SO_ERROR test.
Module['websocket'].emit('error', [sock.stream.fd, sock.error, 'ECONNREFUSED: Connection refused']);
SOCKFS.emit('error', [sock.stream.fd, sock.error, 'ECONNREFUSED: Connection refused']);
};
}
},
Expand Down Expand Up @@ -525,7 +513,7 @@ addToLibrary({
port: sock.sport
// TODO support backlog
});
Module['websocket'].emit('listen', sock.stream.fd); // Send Event with listen fd.
SOCKFS.emit('listen', sock.stream.fd); // Send Event with listen fd.

sock.server.on('connection', function(ws) {
#if SOCKET_DEBUG
Expand All @@ -541,17 +529,17 @@ addToLibrary({

// push to queue for accept to pick up
sock.pending.push(newsock);
Module['websocket'].emit('connection', newsock.stream.fd);
SOCKFS.emit('connection', newsock.stream.fd);
} else {
// create a peer on the listen socket so calling sendto
// with the listen socket and an address will resolve
// to the correct client
SOCKFS.websocket_sock_ops.createPeer(sock, ws);
Module['websocket'].emit('connection', sock.stream.fd);
SOCKFS.emit('connection', sock.stream.fd);
}
});
sock.server.on('close', function() {
Module['websocket'].emit('close', sock.stream.fd);
SOCKFS.emit('close', sock.stream.fd);
sock.server = null;
});
sock.server.on('error', function(error) {
Expand All @@ -562,7 +550,7 @@ addToLibrary({
// occur in a well written app as errors should get trapped in the compiled
// app's own getaddrinfo call.
sock.error = {{{ cDefs.EHOSTUNREACH }}}; // Used in getsockopt for SOL_SOCKET/SO_ERROR test.
Module['websocket'].emit('error', [sock.stream.fd, sock.error, 'EHOSTUNREACH: Host is unreachable']);
SOCKFS.emit('error', [sock.stream.fd, sock.error, 'EHOSTUNREACH: Host is unreachable']);
// don't throw
});
#endif // ENVIRONMENT_MAY_BE_NODE
Expand Down Expand Up @@ -763,7 +751,7 @@ addToLibrary({
// FIXME(sbc): This has no corresponding Pop so will currently keep the
// runtime alive indefinitely.
{{{ runtimeKeepalivePush() }}}
Module['websocket']['on'](event, callback ? _callback : null);
SOCKFS.on(event, callback ? _callback : null);
},
emscripten_set_socket_error_callback__deps: ['$_setNetworkCallback'],
emscripten_set_socket_error_callback: (userData, callback) => {
Expand Down
Loading