Skip to content

fix: properly handle heartbeat timeouts (#95)#132

Merged
carlhoerberg merged 1 commit intomainfrom
fix-heartbeat-timeout
May 28, 2025
Merged

fix: properly handle heartbeat timeouts (#95)#132
carlhoerberg merged 1 commit intomainfrom
fix-heartbeat-timeout

Conversation

@carlhoerberg
Copy link
Copy Markdown
Member

@carlhoerberg carlhoerberg commented Mar 26, 2025

  • Close connection when timeout is detected
  • Properly handle timeout events to disconnect when heartbeats are missed

🤖 Generated with Claude Code

@carlhoerberg carlhoerberg marked this pull request as draft March 26, 2025 20:37
@carlhoerberg carlhoerberg marked this pull request as ready for review May 27, 2025 06:30
Comment thread src/amqp-socket-client.ts
@antondalgren antondalgren requested a review from Copilot May 27, 2025 07:30
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses heartbeat timeout handling by properly closing the connection when a timeout event occurs and by managing socket timeouts based on heartbeat settings.

  • Updated the AMQP client's timeout event handler to trigger disconnection on heartbeat timeout.
  • Added a new test to simulate and verify the heartbeat timeout behavior.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
test/test.ts Added a test to simulate and validate heartbeat timeout events.
src/amqp-socket-client.ts Updated the socket timeout event handler to close the connection on heartbeat timeout.

Copy link
Copy Markdown
Contributor

@antondalgren antondalgren left a comment

Choose a reason for hiding this comment

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

I don't understand where the double heartbeat comes from as Copilot stated. Otherwise I think it looks good.

@cloudamqp cloudamqp deleted a comment from Copilot AI May 28, 2025
@carlhoerberg
Copy link
Copy Markdown
Member Author

It originally came from the issue: #95 but it's not correct, it shouldn't be double.

On heartbeat timeout only the next RPC action was rejected, now
`onerror` is called and the socket is closed.

Fixes #95
@carlhoerberg carlhoerberg force-pushed the fix-heartbeat-timeout branch from 38d1c5e to b9d8680 Compare May 28, 2025 09:39
@carlhoerberg carlhoerberg merged commit 1a407f4 into main May 28, 2025
5 checks passed
@carlhoerberg carlhoerberg deleted the fix-heartbeat-timeout branch May 28, 2025 09:40
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.

4 participants