[jwrosewell]

The data update service reports how each update ended through the updateComplete event. On rc, complete called the listeners with emit and no catch. The status is reported from timer, HTTP and fs.watch callbacks, so a listener that throws there is an uncaught exception, which ends the Node process.

The cross-language audit found this. The Data update page, at the fixed commit 67ead698, says in its Errors section that

As this is a background service, any failures will need to be caught and logged, rather than being lost or causing process failures.

and, in the next line,

These messages MUST also be consistent across languages to make life easier for 51Degrees support.

The Errors section at e89679e9, the specifications rc head, now has a row for this error, "An onUpdateCallbacks callback raised an error", at Error with the message "An unhandled error occurred while checking for automatic updates for engine '<Engine type name>'", and says

These messages MUST also be consistent across languages to make life easier for 51Degrees support. They are the messages and levels of the .NET data update service, and each status is the one .NET reports.

and

An error of any other kind is logged at Error with the message in the last row, also with the error attached.

What changed

complete calls each updateComplete listener in turn and logs, at error level, an error one throws, so the other listeners are still called. It calls the raw listeners, so a once listener is removed after it runs, as emit would remove it.

Both update complete callbacks, the updateComplete listeners and the data file's own onUpdateComplete, now log the same text, which is

An unhandled error occurred while checking for automatic updates for engine '<Engine type>'. Error detail: <error>

built by a named callbackErrorText helper from the existing engineName and errorDetail helpers. That is .NET's message for this error, MessageAutoUpdateUnhandledError, which .NET logs with the exception attached (DataUpdateService.cs lines 673 to 683 at b3e90b4). Node's log takes a string, so the cause follows the text as "Error detail". The data file's own callback was already caught, but it logged the bare error, so it is brought to the same text here.

log used to write nothing when the service had no pipeline attached, which is its state from the constructor until a pipeline takes it, so a callback error in that time was lost. It now writes an error to the console, with any licence key masked, as a pipeline with no error listener does.

Tests

pipeline/fiftyone.pipeline.engines/tests/dataUpdateSpecification.test.js holds two tests.

• "a listener that throws is logged and the others still hear" registers a listener that throws and a once listener, then makes a memory update. The update returns, the once listener hears the status once, and the error is logged once with the text above.
• "an error from the data file callback is logged in the same words" gives the data file an onUpdateComplete that throws and checks the same text, which covers the second exit path of complete.

The listener catch, first round, on 41e3b66:

• Red, the test on rc code (1 of 62 fails, "The listener failed." thrown out of updateFromMemory): https://github.com/51Degrees/51degrees-node/actions/runs/42896022555
• Green, lint and the pipeline engines tests on 41e3b66 (62 passed): https://github.com/51Degrees/51degrees-node/actions/runs/42896064441
• Full build on 41e3b66, success: https://github.com/51Degrees/51degrees-node/actions/runs/42897791416

The callback message, second round:

• Red, on the tests-only commit 9993836, which expects the message above against the previous bare error. 2 of 35 fail in dataUpdateSpecification.test.js, being the two tests above: https://github.com/51Degrees/51degrees-node/actions/runs/42980185833

• d396860 corrects the helper's comment, which had said every language uses this wording. Python does, and .NET has no message naming the callback.

• rc is merged in at b596a1d, which brings the current cxx submodule.

• Full build on b596a1d, success. Configure and the Ubuntu, Ubuntu ARM and Windows jobs on Node 22 and 24 pass, Configure printed "Left out 2 macOS entries", and all 7 Setting 'PullRequestSha' lines are b596a1d489c3c8d3a09b1784692118f04ee3f02c: https://github.com/51Degrees/51degrees-node/actions/runs/42981181357
  The .NET wording and the console, third round:

• Red, run 42995720871 on branch tmp/node-65-red2, the tests-only commit 16035e3 over b596a1d's code. 3 of 36 fail in dataUpdateSpecification.test.js, being the two tests above, which now expect .NET's text, and "a callback error with no pipeline is written to the console".

• Green and full build on the head 24993da, success, run 42991393989. Configure and the Ubuntu, Ubuntu ARM and Windows jobs on Node 22 and 24 pass, Configure printed "Left out 2 macOS entries", and all 7 Setting 'PullRequestSha' lines are 24993da016ab49eb0bd22939cadcdc504f217761.

macOS was left out of this draft's full build under the 9 November decision. The rc head is built with macOS before it merges into main.

Not done here

51degrees-python#69, 51degrees-python#98 and node#69 carried the earlier callback wording and move to the same .NET text in their own drafts.