Skip to content
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.

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

fix(102): Clear accumulated message between HL7 messages/requests. #110

Conversation

alwhiting
Copy link
Contributor

@alwhiting alwhiting commented Oct 15, 2024

I believe this addresses #102
I see there was some problem with PDF file tests but so far all tests pass for me and this has been working in my load testing.

It creates a copy of the loadedMessagse buffer/accumulator to avoid racy behavior when calling the handlers incase they are async. Then as only the copy is needed for handling it immediately clears the accumulator. The copy will increase memory usage a bit but it's a safe option.

Also added the ability to override port for tests since I had stuff running on :3000 and it was easier than stopping it.
Usage: TEST_PORT=19999 npm test (or just omitting it defaults to 3000 as before)

Search terms

Questioner

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

Other things:

  • Write a unit test or update unit tests that cover your change in the code?
  • Set the PR to merge into the develop branch?
  • Clear documentation per the guidelines in the README.md as we are support medical applications?

Bugs5382 and others added 15 commits June 25, 2024 09:01
# [2.5.0](Bugs5382/node-hl7-server@v2.4.1...v2.5.0) (2024-08-29)

### Features

* **Bugs5382#100:** added AE support ([20f33c3](Bugs5382@20f33c3)), closes [Bugs5382#100](Bugs5382#100)
* **Bugs5382#100:** support ae type in send response ([Bugs5382#101](Bugs5382#101)) ([5e09595](Bugs5382@5e09595)), closes [Bugs5382#100](Bugs5382#100)
* develop ([Bugs5382#104](Bugs5382#104)) ([b38334b](Bugs5382@b38334b))
# [3.0.0-beta.1](Bugs5382/node-hl7-server@v2.5.0-beta.1...v3.0.0-beta.1) (2024-09-21)

* 103 feat allow overriding response msh content ([Bugs5382#107](Bugs5382#107)) ([ba7ec92](Bugs5382@ba7ec92))

### Features

* Allow overriding response MSH content [Bugs5382#103](Bugs5382#103) ([b208429](Bugs5382@b208429))
* Allow overriding response MSH content [Bugs5382#103](Bugs5382#103) ([Bugs5382#106](Bugs5382#106)) ([1dcef70](Bugs5382@1dcef70))

### BREAKING CHANGES

* MSH Override Changes
* MSH Override Changes
# [3.0.0](Bugs5382/node-hl7-server@v2.5.0...v3.0.0) (2024-09-21)

* 103 feat allow overriding response msh content ([Bugs5382#107](Bugs5382#107)) ([ba7ec92](Bugs5382@ba7ec92))

### Features

* 3.0.0 ([Bugs5382#108](Bugs5382#108)) ([1252e23](Bugs5382@1252e23))
* Allow overriding response MSH content [Bugs5382#103](Bugs5382#103) ([b208429](Bugs5382@b208429))
* Allow overriding response MSH content [Bugs5382#103](Bugs5382#103) ([Bugs5382#106](Bugs5382#106)) ([1dcef70](Bugs5382@1dcef70))

### BREAKING CHANGES

* MSH Override Changes
* MSH Override Changes
@alwhiting alwhiting requested a review from Bugs5382 as a code owner October 15, 2024 14:47
@alwhiting alwhiting changed the title Clear accumulated message between HL7 messages/requests. fix(102): Clear accumulated message between HL7 messages/requests. Oct 15, 2024
@alwhiting
Copy link
Contributor Author

If you have a different PDF test that this doesn't pass please share it I'm happy to try and do some more work!

@Bugs5382 Bugs5382 self-assigned this Nov 1, 2024
@Bugs5382
Copy link
Owner

Bugs5382 commented Nov 1, 2024

@alwhiting I am checking this out right now.

@Bugs5382 Bugs5382 changed the base branch from main to 102-fix-loadedmessage-is-not-cleared-after-messages-handled November 1, 2024 00:15
Copy link
Owner

@Bugs5382 Bugs5382 left a comment

Choose a reason for hiding this comment

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

lgtm

@Bugs5382 Bugs5382 merged commit ae7d002 into Bugs5382:102-fix-loadedmessage-is-not-cleared-after-messages-handled Nov 1, 2024
1 check passed
github-actions bot pushed a commit that referenced this pull request Nov 1, 2024
## [3.0.1-beta.1](v3.0.0...v3.0.1-beta.1) (2024-11-01)

### Bug Fixes

* 102 fix loadedmessage is not cleared after messages handled ([#111](#111)) ([59cf172](59cf172))
* **102:** Clear accumulated message between HL7 messages/requests.  ([#110](#110)) ([ae7d002](ae7d002))
Copy link
Contributor

github-actions bot commented Nov 1, 2024

🎉 This PR is included in version 3.0.1-beta.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants