Skip to content

feat: new proxy api#72

Merged
alex-sandrk merged 1 commit into
devfrom
feat/new-proxy
Sep 13, 2023
Merged

feat: new proxy api#72
alex-sandrk merged 1 commit into
devfrom
feat/new-proxy

Conversation

@alex-sandrk
Copy link
Copy Markdown
Contributor

No description provided.

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

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.

Comment on lines 20 to 33

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)
Copy link
Copy Markdown

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.

Comment on lines +55 to 66
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
Copy link
Copy Markdown

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.

@alex-sandrk alex-sandrk merged commit 877c41c into dev Sep 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant