feat: adds try/catch block to increase resilience #666

ankur0904 · 2023-12-22T16:46:26Z

Description

Added try/catch block to the http adapter

Related issue(s)

Fixes #640

ankur0904 · 2023-12-22T16:47:48Z

@KhudaDad414
Please have a look :)

src/adapters/http/client.ts

ankur0904 · 2023-12-27T19:20:27Z

Hi @KhudaDad414 I made the suggested changes. Please have a look and give your valuable suggestions.

KhudaDad414 · 2023-12-28T11:16:46Z

@ankur0904 can you reset the auto format changes? it's hard to know what changes you have made.

secondly the try/catch block should only be inconnect() method, and connect() should call _connect() inside try block. same with the send() method. send() method should have a try/catch block calling _send().

this is what I mean:

class Adapter {
    // This is your main public method for establishing a connection
    public async connect() {
        try {
            await this._connect();
        } catch (error) {
            // Handle or rethrow the error as needed
        }
    }

    // The actual connection logic is encapsulated here
    private async _connect() {
        // Your connection logic goes here
    }

    // The public method for sending data
    public async send() {
        try {
            await this._send(data);
        } catch (error) {
            // Handle error
        }
    }

    // The actual sending logic is handled privately here
    private async _send() {
        // Your sending logic goes here
    }
}

ankur0904 · 2024-01-04T18:21:11Z

@KhudaDad414 I tried to reset the auto format change. I have undo the last commit, removed the auto format manually, then recommitted the changes, got the merge conflict, and resolved the merge conflict.

I'll take your code as a reference and update the PR soon.

Thanks

coveralls · 2024-01-05T16:18:02Z

Pull Request Test Coverage Report for Build 7558836827

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 72.884%

Totals
Change from base Build 7540195680:	0.0%
Covered Lines:	388
Relevant Lines:	483

💛 - Coveralls

KhudaDad414 · 2024-01-08T09:06:58Z

src/adapters/cluster/redis/index.ts

  }

  async _connect(): Promise<this> {
+    try {


we don't want to catch the error since we left the error handling the _connect method. errors thrown by this function will be handled by the connect method.

Suggested change

try {

KhudaDad414 · 2024-01-08T09:07:11Z

src/adapters/cluster/redis/index.ts

+    } catch (error) {
+      console.error('Error connecting to Redis:', error)
+      throw error
+    }


Suggested change

} catch (error) {

console.error('Error connecting to Redis:', error)

throw error

}

KhudaDad414 · 2024-01-08T09:09:00Z

src/adapters/http/server.ts

@@ -164,6 +174,7 @@ class HttpAdapter extends Adapter {
  }

  async _connect(): Promise<this> {
+    try {


Suggested change

try {

KhudaDad414 · 2024-01-08T09:09:16Z

src/adapters/http/server.ts

+    } catch (error) {
+      console.error(error)
+      throw error
+    }


Suggested change

} catch (error) {

console.error(error)

throw error

}

KhudaDad414 · 2024-01-08T09:11:02Z

src/adapters/kafka/index.ts

-    const scramSha512SecurityReq = securityRequirements.find(
-      (sec) => sec.type() === 'scramSha512'
-    )
+  async _connect() {


you have removed the public method. the functionality won't work anymore. connect method is used for error handling and _connect for the actual logic. we always call the _connect method from the connect method.

KhudaDad414 · 2024-01-08T09:12:18Z

src/adapters/mqtt/index.ts

+    } catch (error) {
+      console.error(error)
+      throw error
+    }


Suggested change

} catch (error) {

console.error(error)

throw error

}

KhudaDad414 · 2024-01-08T09:12:35Z

src/adapters/socket.io/index.ts

  }

  async _connect(): Promise<this> {
+    try {


Suggested change

try {

KhudaDad414 · 2024-01-08T09:12:45Z

src/adapters/socket.io/index.ts

+    } catch (error) {
+      console.error('An error occurred while connecting:', error)
+      throw error
+    }


Suggested change

} catch (error) {

console.error('An error occurred while connecting:', error)

throw error

}

KhudaDad414 · 2024-01-08T09:13:05Z

src/adapters/ws/client.ts

  }

  private async _connect(): Promise<this> {
+    try{


Suggested change

try{

KhudaDad414 · 2024-01-08T09:13:14Z

src/adapters/ws/client.ts

+    }catch (error) {
+      console.error('An error occurred while connecting:', error)
+      throw error
+    }


Suggested change

}catch (error) {

console.error('An error occurred while connecting:', error)

throw error

}

ankur0904 · 2024-01-08T11:41:36Z

Sorry @KhudaDad414 for making the mistake. Now I fully understand that connect -> used for calling the _connect and handing the error & _connect for the business logic.

ankur0904 · 2024-01-08T11:43:12Z

I request you to please review the changes in src/adapters/kafka/index.ts this file.

KhudaDad414 · 2024-01-08T14:35:33Z

I request you to please review the changes in src/adapters/kafka/index.ts this file.

since this file doesn't have a _connect method, I suggest you move the business logic in _connect and them handle the error in the connect method. just like other adapters.

ankur0904 · 2024-01-09T13:15:13Z

@KhudaDad414 I added the connect function but I have some doubts about _connect function. Please have a look at the src/adapters/kafka/index.ts _connect function.

KhudaDad414

Left some comments.

KhudaDad414 · 2024-01-09T15:25:03Z

src/adapters/cluster/redis/index.ts

    return this._send(message)
+    } catch (error) {
+      console.error('Error sending message to Redis:', error)
+      throw error


Suggested change

throw error

KhudaDad414 · 2024-01-09T15:26:54Z

src/adapters/cluster/redis/index.ts

    return this._send(message)
+    } catch (error) {
+      console.error('Error sending message to Redis:', error)


Suggested change

console.error('Error sending message to Redis:', error)

const errorMessage = `Failed to send message on channel '${message.channel}' to server '${message.serverName}'`;

this.emit('error', new Error(errorMessage));

this.emit('error', error)

KhudaDad414 · 2024-01-09T15:29:18Z

src/adapters/http/client.ts

+      })
+      return this
+    } catch (err) {
+      logWarningMessage(`Failed to Connect: An error occurred while connecting to '${this.name()}' on the '${this.channelNames}' channel. Please review the error details below for further information and corrective action.`)


Suggested change

logWarningMessage(`Failed to Connect: An error occurred while connecting to '${this.name()}' on the '${this.channelNames}' channel. Please review the error details below for further information and corrective action.`)

logWarningMessage(`Failed to Connect: An error occurred while connecting to '${this.name()}'. Please review the error details below for further information and corrective action.`)

KhudaDad414 · 2024-01-09T15:30:38Z

src/adapters/http/server.ts

+    try{
+      return this._send(message)
+    } catch (error) {
+      console.error('Error sending message to HTTP:', error)


also add the channel and the server name to the error so it becomes easier to find the error.

KhudaDad414 · 2024-01-09T15:31:43Z

src/adapters/mqtt/index.ts

+    try{
+      return this._send(message)
+    } catch(error){
+      console.error('Error sending message to MQTT:', error)


just like my previouse suggestion, please add the channel and the server where the error occured.

KhudaDad414 · 2024-01-09T15:31:53Z

src/adapters/socket.io/index.ts

+    try{
+      return this._send(message)
+    } catch(error){
+      console.error('Error sending message to Socket.IO:', error)


KhudaDad414 · 2024-01-09T15:32:00Z

src/adapters/ws/client.ts

+    try{
+      return this._send(message)
+    } catch(error){
+      console.error('Error sending message to WS:', error)


KhudaDad414 · 2024-01-12T12:00:44Z

@ankur0904 do you need some help here?

ankur0904 · 2024-01-13T17:36:37Z

I'm so sorry @KhudaDad414 for the late response. I have made the required changes in the latest commit.

Thanks

KhudaDad414

LGTM!

KhudaDad414 · 2024-01-15T10:29:36Z

@ankur0904 the tests are failing.

ankur0904 · 2024-01-17T16:40:11Z

@KhudaDad414 Please look at the updated changes.

KhudaDad414 · 2024-01-18T09:57:33Z

Hey @ankur0904 the SonarCloud is complaining about the code duplication. And it has a point. all of the code here is duplicated. 😆

let's make it simpler.
instead of handling the error in each adapter let's handle it where the connect and send functions are called.

for connect method:
https://github.com/KhudaDad414/glee/blob/bb511f62e332d6c0fa8510079ca2e10564c65104/src/lib/glee.ts#L154-L161

for send method:
https://github.com/KhudaDad414/glee/blob/bb511f62e332d6c0fa8510079ca2e10564c65104/src/lib/glee.ts#L280
https://github.com/KhudaDad414/glee/blob/bb511f62e332d6c0fa8510079ca2e10564c65104/src/lib/glee.ts#L217

ankur0904 · 2024-01-18T16:34:36Z

@KhudaDad414 I am not sure what to do with send method:
https://github.com/KhudaDad414/glee/blob/bb511f62e332d6c0fa8510079ca2e10564c65104/src/lib/glee.ts#L280
https://github.com/KhudaDad414/glee/blob/bb511f62e332d6c0fa8510079ca2e10564c65104/src/lib/glee.ts#L217

KhudaDad414 · 2024-01-23T14:24:41Z

@ankur0904 just wrap the send inside a try/catch block and handle the error.

sonarcloud · 2024-01-24T11:33:41Z

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

ankur0904 · 2024-01-24T12:01:39Z

Hey @ankur0904 the SonarCloud is complaining about the code duplication. And it has a point. all of the code here is duplicated. 😆

let's make it simpler. instead of handling the error in each adapter let's handle it where the connect and send functions are called.

for connect method: https://github.com/KhudaDad414/glee/blob/bb511f62e332d6c0fa8510079ca2e10564c65104/src/lib/glee.ts#L154-L161

for send method: https://github.com/KhudaDad414/glee/blob/bb511f62e332d6c0fa8510079ca2e10564c65104/src/lib/glee.ts#L280 https://github.com/KhudaDad414/glee/blob/bb511f62e332d6c0fa8510079ca2e10564c65104/src/lib/glee.ts#L217

@KhudaDad414
for send method:
error is already handled:
e.g.,

 a.instance.send(msg).catch((e: Error) => {
              this._processError(errorMiddlewares, e, msg)
})

&

this._clusterAdapter.instance.send(message).catch((e: Error) => {
        this._processError(this._router.getErrorMiddlewares(), e, message)
})

KhudaDad414

LGTM

ankur0904 · 2024-01-27T13:40:57Z

@KhudaDad414
Thanks for your valuable guidance throughout the process. Happy to work on another issue as well.

KhudaDad414 · 2024-03-18T10:38:14Z

/rtm

asyncapi-bot · 2024-03-18T10:42:01Z

🎉 This PR is included in version 0.36.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Adds try/catch block to increase resilience

ad99d53

ankur0904 requested review from fmvilas, Souvikns, KhudaDad414 and asyncapi-bot-eve as code owners December 22, 2023 16:46

KhudaDad414 requested changes Dec 27, 2023

View reviewed changes

src/adapters/http/client.ts Show resolved Hide resolved

Made the suggested changes

178bf6f

ankur0904 force-pushed the increase-resilience branch from 824bac6 to 178bf6f Compare January 4, 2024 18:23

Merge branch 'asyncapi:master' into increase-resilience

1ec5a69

made the required changes

159f3b6

KhudaDad414 requested changes Jan 8, 2024

View reviewed changes

Removes the unnecessary try catch blocks

a8affed

Adds connect function

d73a9c8

KhudaDad414 requested changes Jan 9, 2024

View reviewed changes

Updates the error message to provide more detail

0d6c955

KhudaDad414 previously approved these changes Jan 15, 2024

View reviewed changes

KhudaDad414 self-requested a review January 15, 2024 10:30

fix failing test

ae99256

ankur0904 dismissed KhudaDad414’s stale review via ae99256 January 17, 2024 16:33

ankur0904 force-pushed the increase-resilience branch from fbf2b99 to ae99256 Compare January 17, 2024 16:33

ankur0904 added 2 commits January 18, 2024 21:35

Merge branch 'asyncapi:master' into increase-resilience

7c57144

Remove duplicacy

f0f44f4

Merge branch 'asyncapi:master' into increase-resilience

3123107

KhudaDad414 approved these changes Jan 25, 2024

View reviewed changes

asyncapi-bot added the ready-to-merge label Mar 18, 2024

asyncapi-bot merged commit 485068c into asyncapi:master Mar 18, 2024
11 checks passed

asyncapi-bot added the released label Mar 18, 2024

-      console.error('Error sending message to Redis:', error)
+      const errorMessage = `Failed to send message on channel '${message.channel}' to server '${message.serverName}'`;
+      this.emit('error', new Error(errorMessage));
+      this.emit('error', error)

	logWarningMessage(`Failed to Connect: An error occurred while connecting to '${this.name()}' on the '${this.channelNames}' channel. Please review the error details below for further information and corrective action.`)
	logWarningMessage(`Failed to Connect: An error occurred while connecting to '${this.name()}'. Please review the error details below for further information and corrective action.`)

feat: adds try/catch block to increase resilience #666

feat: adds try/catch block to increase resilience #666

Conversation

ankur0904 commented Dec 22, 2023

ankur0904 commented Dec 22, 2023

ankur0904 commented Dec 27, 2023

KhudaDad414 commented Dec 28, 2023

ankur0904 commented Jan 4, 2024

coveralls commented Jan 5, 2024 • edited Loading

Pull Request Test Coverage Report for Build 7558836827

💛 - Coveralls

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ankur0904 commented Jan 8, 2024

ankur0904 commented Jan 8, 2024

KhudaDad414 commented Jan 8, 2024

ankur0904 commented Jan 9, 2024

KhudaDad414 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KhudaDad414 commented Jan 12, 2024

ankur0904 commented Jan 13, 2024

KhudaDad414 left a comment

Choose a reason for hiding this comment

KhudaDad414 commented Jan 15, 2024 • edited Loading

ankur0904 commented Jan 17, 2024

KhudaDad414 commented Jan 18, 2024 • edited Loading

ankur0904 commented Jan 18, 2024 • edited Loading

KhudaDad414 commented Jan 23, 2024

sonarcloud bot commented Jan 24, 2024

Quality Gate passed

ankur0904 commented Jan 24, 2024 • edited Loading

KhudaDad414 left a comment

Choose a reason for hiding this comment

ankur0904 commented Jan 27, 2024

KhudaDad414 commented Mar 18, 2024

asyncapi-bot commented Mar 18, 2024

coveralls commented Jan 5, 2024 •

edited

Loading

KhudaDad414 commented Jan 15, 2024 •

edited

Loading

KhudaDad414 commented Jan 18, 2024 •

edited

Loading

ankur0904 commented Jan 18, 2024 •

edited

Loading

ankur0904 commented Jan 24, 2024 •

edited

Loading