kmcginnes commented on code in PR #2506:
URL: https://github.com/apache/tinkerpop/pull/2506#discussion_r1506824337
##########
gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/connection.js:
##########
@@ -230,92 +213,82 @@ class Connection extends EventEmitter {
};
const request_buf = this._writer.writeRequest(request);
- const message = Buffer.concat([this._header_buf, request_buf]);
+ const message = utils.toArrayBuffer(Buffer.concat([this._header_buf,
request_buf]));
this._ws.send(message);
})
.catch((err) => readableStream.destroy(err));
return readableStream;
}
- _getDefaultReader(mimeType) {
+ #getDefaultReader(mimeType) {
if (mimeType === graphBinaryMimeType) {
return graphBinaryReader;
}
return mimeType === graphSON2MimeType ? new serializer.GraphSON2Reader() :
new serializer.GraphSONReader();
}
- _getDefaultWriter(mimeType) {
+ #getDefaultWriter(mimeType) {
if (mimeType === graphBinaryMimeType) {
return graphBinaryWriter;
}
return mimeType === graphSON2MimeType ? new serializer.GraphSON2Writer() :
new serializer.GraphSONWriter();
}
- _pingHeartbeat() {
- if (this._pingInterval) {
- clearInterval(this._pingInterval);
- this._pingInterval = null;
- }
+ #handleOpen = () => {
+ this._openPromise.resolve();
+ this.isOpen = true;
+ };
- this._pingInterval = setInterval(() => {
- if (this.isOpen === false) {
- // in case of if not open..
- if (this._pingInterval) {
- clearInterval(this._pingInterval);
- this._pingInterval = null;
- }
- }
-
- this._pongTimeout = setTimeout(() => {
- this._ws.terminate();
- }, this._pongTimeoutDelay);
-
- this._ws.ping();
- }, this._pingIntervalDelay);
- }
-
- _handleError(err) {
- this.emit('log', `ws error ${err}`);
- this._cleanupWebsocket(err);
- this.emit('socketError', err);
- }
+ /**
+ * @param {Event} event
+ */
+ #handleError = ({ error }) => {
+ this._openPromise.reject(error);
+ this.emit('log', `ws error ${error}`);
+ this.#cleanupWebsocket(error);
+ this.emit('socketError', error);
+ };
- _handleClose(code, message) {
+ /**
+ * @param {CloseEvent} event
+ */
+ #handleClose = ({ code, message }) => {
this.emit('log', `ws close code=${code} message=${message}`);
- this._cleanupWebsocket();
+ this.#cleanupWebsocket();
if (this._closeCallback) {
this._closeCallback();
}
this.emit('close', code, message);
- }
+ };
+
+ /**
+ * @param {MessageEvent<any>} event
+ */
+ #handleMessage = ({ data: _data }) => {
+ const data = _data instanceof ArrayBuffer ? Buffer.from(_data) : _data;
- _handleMessage(data) {
const response = this._reader.readResponse(data);
if (response.requestId === null || response.requestId === undefined) {
// There was a serialization issue on the server that prevented the
parsing of the request id
// We invoke any of the pending handlers with an error
Object.keys(this._responseHandlers).forEach((requestId) => {
const handler = this._responseHandlers[requestId];
- this._clearHandler(requestId);
+ this.#clearHandler(requestId);
if (response.status !== undefined && response.status.message) {
return handler.callback(
// TINKERPOP-2285: keep the old server error message in case folks
are parsing that - fix in a future breaking version
new ResponseError(
- util.format(
- 'Server error (no request information): %s (%d)',
- response.status.message,
- response.status.code,
- ),
+ `Server error (no request information):
${response.status.message} (${response.status.code})`,
response.status,
),
);
}
// TINKERPOP-2285: keep the old server error message in case folks are
parsing that - fix in a future breaking version
return handler.callback(
- new ResponseError(util.format('Server error (no request
information): %j', response), response.status),
+ new ResponseError(`Server error (no request information):
${response}`, response.status),
Review Comment:
@tien One thing to keep in mind with template strings is that they do not
expand objects. They typically print as `[Object object]`.
You can see an example of that here:
https://stackoverflow.com/a/46146887/1558906
One solution could be to use `JSON.stringify()`, but this could potentially
lead to issues with circular references, which is inherent to
`JSON.stringify()`.
My best guess is that the chances of running in to a circular reference in
the `response` object is quite low. In addition, this `ResponseError` is
already a fallback to a more specific case. So I would recommend using
`JSON.stringify()` here to ensure the error message is usable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
