DocumentMigrator: add logging for errors outside of migration functions

[pgayvallet]

Summary

Fix #75055

Add logging of errors occurring from outside of the wrapped migration function to DocumentMigrator.migrate

Checklist

• Unit or functional tests were updated or added to match the most common scenarios

[pgayvallet]

Error from within the wrapped transform functions are currently logged as warnings

kibana/src/core/server/saved_objects/migrations/core/document_migrator.ts

Lines 339 to 341 in 079ef06

	log.warn(
	`Failed to transform document ${doc}. Transform: ${failedTransform}\nDoc: ${failedDoc}`
	);

As the error is actually causing the migration to fail and the server to not start, I wonder if we shouldn't upgrade that to the error level, wdyt?

[joshdover]

Upgrade to error makes sense 👍

[pgayvallet]

The errors were already logged when thrown from within the migration function, via the wrapWithTry wrapper. Only non-logged errors are the one from the actual DocumentMigrator code.

As the migration function errors got some specific infos, I did not remove them when adding this try/catch block. Instead, I use this alreadyLogged flag trick to avoid logging the error twice, while preserving the thrown error.

[elasticmachine]

Pinging @elastic/kibana-platform (Team:Platform)

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 2bd351a

Build metrics

✅ unchanged

History

• 💚 Build #72566 succeeded 2d8ca6d
• 💚 Build #71793 succeeded 079ef06

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

[joshdover]

Code looks good. Only remaining question I have is where were these exceptions being swallowed before? Is it possible that any other exceptions are being swallowed in the same place?

[pgayvallet]

Only remaining question I have is where were these exceptions being swallowed before? Is it possible that any other exceptions are being swallowed in the same place?

Don't even know why I did not look a little deeper, as every uncatched exceptions are supposed to be catched by root

kibana/src/core/server/root/index.ts

Lines 60 to 68 in 200957b

	public async start() {
	this.log.debug('starting root');
	try {
	return await this.server.start();
	} catch (e) {
	await this.shutdown(e);
	throw e;
	}
	}

After a little more investigation, I found the actual culprit:

kibana/src/core/server/saved_objects/migrations/core/migrate_raw_docs.ts

Lines 76 to 87 in 68cf857

	function transformNonBlocking(
	transform: TransformFn
	): (doc: SavedObjectUnsanitizedDoc) => Promise<SavedObjectUnsanitizedDoc> {
	// promises aren't enough to unblock the event loop
	return (doc: SavedObjectUnsanitizedDoc) =>
	new Promise((resolve) => {
	// set immediate is though
	setImmediate(() => {
	resolve(transform(doc));
	});
	});
	}

This recent addition to avoid blocking the event loop during migration does not handle error during the failure call, resulting in an unhandled promise rejection.

[pgayvallet]

Closing in favor of #77018