Skip to content

feat: new proxy api #72

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
alex-sandrk merged 1 commit into from
Sep 13, 2023
Merged

feat: new proxy api #72

Show file tree
Hide file tree
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
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@lumerin/wallet-core",
"version": "1.0.65",
"version": "1.0.66",
"author": {
"name": "Lumerin",
"email": "developer@lumerin.io",
Expand Down
17 changes: 6 additions & 11 deletions src/plugins/proxy-router/connections-manager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,14 @@ function createConnectionsManager(config, eventBus) {

let interval

const getConnections = async (sellerUrl, buyerUrl) => {
const getConnections = async (proxyUrl) => {
const getMiners = async (url) => {
return (await createAxios({ baseURL: url })('/miners')).data?.Miners
}

if (sellerUrl && buyerUrl) {
const sellerMiners = await getMiners(sellerUrl)
const buyerMiners = (await getMiners(buyerUrl)).map((x) => ({
...x,
Status: 'busy',
}))

return [...sellerMiners, ...buyerMiners]
if (proxyUrl) {
const sellerMiners = await getMiners(proxyUrl);
return [...sellerMiners];
}

return await getMiners(proxyRouterUrl)
Comment on lines 20 to 33
Copy link
Copy Markdown

@codereviewbot-ai codereviewbot-ai Bot Sep 13, 2023

Choose a reason for hiding this comment

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

The function getConnections is not handling any potential errors that might occur during the execution of the getMiners function. If createAxios or the subsequent call to /miners fails for any reason (e.g., network issues, server error, etc.), it will throw an error that is not being caught. This could lead to unexpected behavior or crashes in your application. To improve error handling, you should wrap the contents of getConnections in a try-catch block and handle any errors appropriately, such as logging the error and returning a default value or rethrowing the error to be handled by the caller.

Expand All @@ -57,15 +52,15 @@ function createConnectionsManager(config, eventBus) {
*
* @returns {object} The event emitter.
*/
function getConnectionsStream(sellerUrl, buyerUrl) {
function getConnectionsStream(proxyUrl) {
const stream = new EventEmitter()

let isConnected = false

disconnect()
interval = setInterval(async () => {
try {
const connections = await getConnections(sellerUrl, buyerUrl)
const connections = await getConnections(proxyUrl)

if (!isConnected) {
isConnected = true
Comment on lines +55 to 66
Copy link
Copy Markdown

@codereviewbot-ai codereviewbot-ai Bot Sep 13, 2023

Choose a reason for hiding this comment

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

The function getConnectionsStream is using a global variable interval to store the interval ID from setInterval. This could potentially lead to issues if this function is called multiple times, as it would overwrite the previous interval ID, making it impossible to clear the previous interval. This could lead to memory leaks and unexpected behavior. To avoid this, you should consider storing the interval ID in a closure or in an object that is tied to the lifecycle of the relevant component.

Also, the function disconnect is called without being defined or imported anywhere in the provided code. If it's not defined elsewhere in the codebase, this will result in a ReferenceError at runtime. Make sure that disconnect is properly defined and available in this scope.

Expand Down
2 changes: 1 addition & 1 deletion src/plugins/proxy-router/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ function createPlugin() {

const refreshConnectionsStream = (data) =>
connectionManager
.getConnectionsStream(data.sellerNodeUrl, data.buyerNodeUrl)
.getConnectionsStream(data.proxyNodeUrl)
.on('data', (data) => {
eventBus.emit('proxy-router-connections-changed', {
connections: data.connections,
Comment on lines 16 to 21
Copy link
Copy Markdown

@codereviewbot-ai codereviewbot-ai Bot Sep 13, 2023

Choose a reason for hiding this comment

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

The function refreshConnectionsStream is not handling any potential errors that might occur during the execution of getConnectionsStream. This could lead to unhandled exceptions or silent failures, making it difficult to debug and maintain the code.

To improve this, you should add an error event listener to the stream returned by getConnectionsStream. In the error event handler, you can emit an event on the eventBus to notify other parts of your application that an error has occurred. This way, you can handle the error gracefully and provide feedback to the user or log the error for debugging purposes.

Expand Down