Skip to content

fix!: resolve memory leaks in receiveMessage and response body handling#8

Merged
fgiova merged 1 commit into
mainfrom
fix/memory-leaks
Feb 12, 2026
Merged

fix!: resolve memory leaks in receiveMessage and response body handling#8
fgiova merged 1 commit into
mainfrom
fix/memory-leaks

Conversation

@fgiova
Copy link
Copy Markdown
Owner

@fgiova fgiova commented Feb 12, 2026

  • Cache receiveMessage clients instead of creating/closing one per call, preventing connection churn and potential leaks
  • Dump response body on non-JSON responses to avoid undrained socket leaks
  • Destroy cached receiveMessage clients in destroy() method
  • Drop Node.js 20 support, require ^22.14.0 || >= 24.10.0
  • Upgrade @fgiova/aws-signature to ^4.0.0, undici to ^7.21.0
  • Update CI matrix and release workflow (Node 22/24 only, OIDC permissions)
  • Fix README badges, usage example and API signatures

BREAKING-CHANGE: Drop support to Node.js 20.x

Copilot AI review requested due to automatic review settings February 12, 2026 14:54
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 connection/body handling leaks by reusing receiveMessage clients, draining non-JSON response bodies, and aligning project/runtime/tooling with Node 22/24.

Changes:

  • Cache receiveMessage HTTP clients and destroy them in MiniSQSClient.destroy()
  • Drain response bodies for non-JSON responses to avoid socket leaks
  • Drop Node 20 support and update dependencies/CI/docs accordingly

Reviewed changes

Copilot reviewed 6 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/index.ts Adds a cached undici Client for receiveMessage, drains bodies on non-JSON responses, and destroys cached clients on destroy()
test/index.ts Adds a test intended to cover cached receiveMessage client behavior
package.json Updates Node engine range and bumps @fgiova/aws-signature + undici versions
README.md Updates badges, Node requirements, and usage/API signature examples
.nvmrc Updates default Node version to 22
.gitignore Ignores /.claude/
.github/workflows/node.js.yml Updates CI matrix to Node 22/24 and adjusts permissions/coverage/release steps

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/index.ts
Comment thread test/index.ts
Comment on lines 72 to +79
release:
runs-on: ubuntu-latest
needs: [test, build]
if: github.event_name == 'push' # Release only on Push
permissions:
contents: write # to be able to publish a GitHub release
issues: write # to be able to comment on released issues
pull-requests: write # to be able to comment on released pull requests
id-token: write # to enable use of OIDC for trusted publishing and npm provenance
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

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

The release job no longer has the push-only guard (previously if: github.event_name == 'push'). Since this workflow also runs on pull_request, the release job can execute on PR events, which is risky for publishing. Reintroduce an if: condition (e.g., github.event_name == 'push' and/or github.ref == 'refs/heads/main') to ensure releases only run on intended events/branches.

Copilot uses AI. Check for mistakes.
Comment thread README.md
- Cache receiveMessage clients instead of creating/closing one per call, preventing connection churn and potential leaks
- Dump response body on non-JSON responses to avoid undrained socket leaks
- Destroy cached receiveMessage clients in destroy() method
- Drop Node.js 20 support, require ^22.14.0 || >= 24.10.0
- Upgrade @fgiova/aws-signature to ^4.0.0, undici to ^7.21.0
- Update CI matrix and release workflow (Node 22/24 only, OIDC permissions)
- Fix README badges, usage example and API signatures

BREAKING-CHANGE: Drop support to Node.js 20.x
@fgiova fgiova merged commit b9bc16a into main Feb 12, 2026
5 checks passed
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.

2 participants