fix: out bound push fails while writing to info doc #9855

jonathanbataire · 2025-03-24T08:23:25Z

Description

When two out bound pushes running against the same document (same doc id) there is an uncaught error as shown below while writing to the sentinel info doc that leads to duplication in the push.

while looping thru the outbound keys eg push-1 and push-2,
push-1 will successfully write the info doc for the report
when it get to push-2 it attempts to put the info doc but runs into a conflict error
This will lead to creation of an unnecessary task:outbound which leads to push-2 being pushed again.

Fixes #9854

{
  error: 'conflict',
  reason: 'Document update conflict.',
  status: 409,
  name: 'conflict',
  message: 'Document update conflict.',
  stack: 'Error\n' +
    '    at Object.generateErrorFromResponse (/home/jonathan/Downloads/cht-core-4.9.0/sentinel/node_modules/pouchdb-errors/lib/index.js:104:18)\n' +
    '    at /home/jonathan/Downloads/cht-core-4.9.0/sentinel/node_modules/pouchdb-adapter-http/lib/index.js:254:33\n' +
    '    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)',
  docId: 'c4628ff1-e3ac-4a2c-a499-f2f5fe8d81b6-info'
}

Code review checklist

UI/UX backwards compatible: Test it works for the new design (enabled by default). And test it works in the old design, enable can_view_old_navigation permission to see the old design. Test it has appropriate design for RTL languages.
Readable: Concise, well named, follows the style guide, documented if necessary.
Documented: Configuration and user documentation on cht-docs
Tested: Unit and/or e2e where appropriate
Internationalised: All user facing text
Backwards compatible: Works with existing data and configuration or includes a migration. Any breaking changes documented in the release notes.

License

The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.

dianabarsan · 2025-03-26T11:20:30Z

@jonathanbataire I'm happy to review this when it's ready! Please let me know :)

jonathanbataire · 2025-03-26T15:52:53Z

ready for review @dianabarsan

dianabarsan

Really appreciate you resolving on this! I left one suggestion inline.

dianabarsan · 2025-03-28T12:11:34Z

shared-libs/transitions/src/transitions/mark_for_outbound.js

@@ -48,8 +48,17 @@ const markForOutbound = (change) => {
      .then(sent => {
        if (sent) {
          // Successfully sent, outbound.send wrote to the infodoc
-
-          return db.sentinel.put(change.info);
+          return db.sentinel.get(change.info._id)


I think this conflict resolution is a bit verbose to be here, and it doesn't treat an eventual conflict directly - in your error, the status that the PUT request got was 409, and here you're only resolving for 404s (not found).

I think the best solution here is to use the infodoc shared lib, which already has infodoc conflct resolution built in and already has a saveCompletedTasks function (https://github.com/medic/cht-core/blob/master/shared-libs/infodoc/src/infodoc.js#L285). This function doesn't receive parameters now, but that can be changed.

Please let me know what you think!

dianabarsan · 2025-03-28T12:18:30Z

tests/integration/sentinel/transitions/mark-for-outbound.spec.js

@@ -133,6 +133,78 @@ describe('mark_for_outbound', () => {
        });
    });

+    it('correctly creates and sends multiple outbound requests for the same report immediately', () => {


Love this test!

dianabarsan · 2025-03-31T13:40:11Z

shared-libs/infodoc/src/infodoc.js

@@ -156,7 +156,7 @@ const saveTransitions = change => {
 };

 const saveCompletedTasks = (id, infodoc) => {


I think you need to add a parameter here, to save correct completed_tasks, instead of just completing all tasks.

jonathanbataire · 2025-04-01T10:48:17Z

shared-libs/infodoc/src/infodoc.js

@@ -155,8 +155,8 @@ const saveTransitions = change => {
  return saveProperty(change.id, change.info, 'transitions', {});
 };

-const saveCompletedTasks = (id, infodoc) => {
-  return saveProperty(id, infodoc, 'completed_tasks', {});
+const saveCompletedTasks = (id, infodoc, completedTasks = []) => {


Oh... now that I check the implementation, I see that the property value was taken from the infodoc that was already passed, my bad!

dianabarsan

Awesome!

dianabarsan · 2025-04-02T09:00:35Z

shared-libs/infodoc/src/infodoc.js

@@ -155,8 +155,8 @@ const saveTransitions = change => {
  return saveProperty(change.id, change.info, 'transitions', {});
 };

-const saveCompletedTasks = (id, infodoc) => {
-  return saveProperty(id, infodoc, 'completed_tasks', {});
+const saveCompletedTasks = (id, infodoc, completedTasks = []) => {


Oh... now that I check the implementation, I see that the property value was taken from the infodoc that was already passed, my bad!

chore: merge branch master

4c8d02b

jonathanbataire force-pushed the outbound-bug branch from d4cb07e to 4c8d02b Compare March 25, 2025 06:51

jonathanbataire changed the title ~~Out bound push fails while writing to info doc~~ fix: Out bound push fails while writing to info doc Mar 25, 2025

jonathanbataire changed the title ~~fix: Out bound push fails while writing to info doc~~ fix: out bound push fails while writing to info doc Mar 25, 2025

jonathanbataire added 6 commits March 26, 2025 15:51

add unit test

0a239d2

clean up tests

01e19f2

fix mark_for_outbound test

48d37ea

clean up

a0a528d

improve outbound test

506248a

clean up 2

8b55b01

dianabarsan self-requested a review March 26, 2025 15:59

jonathanbataire added 4 commits March 27, 2025 08:58

Merge branch 'master' into outbound-bug

f106a98

improve test name

4784eec

clean up writing to info doc

e722f19

Merge branch 'master' into outbound-bug

72b37ac

dianabarsan requested changes Mar 28, 2025

View reviewed changes

jonathanbataire added 5 commits March 31, 2025 11:06

improves save completed tasks

97e5116

Merge branch 'master' into outbound-bug

fc41c8a

fix hanging code

72a6646

Merge branch 'master' into outbound-bug

89b3550

clean up

e30a12a

jonathanbataire requested a review from dianabarsan March 31, 2025 13:08

dianabarsan reviewed Mar 31, 2025

View reviewed changes

feedback and clean up

8d0a427

jonathanbataire commented Apr 1, 2025

View reviewed changes

jonathanbataire requested a review from dianabarsan April 1, 2025 10:48

dianabarsan approved these changes Apr 2, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: out bound push fails while writing to info doc #9855

fix: out bound push fails while writing to info doc #9855

jonathanbataire commented Mar 24, 2025 •

edited

Loading

dianabarsan commented Mar 26, 2025

jonathanbataire commented Mar 26, 2025

dianabarsan left a comment

dianabarsan Mar 28, 2025

dianabarsan Mar 28, 2025

dianabarsan Mar 31, 2025

jonathanbataire Apr 1, 2025

dianabarsan Apr 2, 2025

dianabarsan left a comment

dianabarsan Apr 2, 2025

		@@ -156,7 +156,7 @@ const saveTransitions = change => {
		};

		const saveCompletedTasks = (id, infodoc) => {

fix: out bound push fails while writing to info doc #9855

Are you sure you want to change the base?

fix: out bound push fails while writing to info doc #9855

Conversation

jonathanbataire commented Mar 24, 2025 • edited Loading

Description

Code review checklist

License

dianabarsan commented Mar 26, 2025

jonathanbataire commented Mar 26, 2025

dianabarsan left a comment

Choose a reason for hiding this comment

dianabarsan Mar 28, 2025

Choose a reason for hiding this comment

dianabarsan Mar 28, 2025

Choose a reason for hiding this comment

dianabarsan Mar 31, 2025

Choose a reason for hiding this comment

jonathanbataire Apr 1, 2025

Choose a reason for hiding this comment

dianabarsan Apr 2, 2025

Choose a reason for hiding this comment

dianabarsan left a comment

Choose a reason for hiding this comment

dianabarsan Apr 2, 2025

Choose a reason for hiding this comment

jonathanbataire commented Mar 24, 2025 •

edited

Loading