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

OCRVS-8278 null extensions cause reindex-search to fail #8279

Closed
wants to merge 1 commit into from

Conversation

euanmillar
Copy link
Collaborator

Handle null extensions to resolve the bug raised here: #8278

Still getting a TypeScript error "Array.find(<anonymous>)" logging in migration microservice from findExtension function in commons but this fix ensures that the reindex passes and we didnt notice any errors in the data.

Copy link

github-actions bot commented Jan 2, 2025

Oops! Looks like you forgot to update the changelog. When updating CHANGELOG.md, please consider the following:

  • Changelog is read by country implementors who might not always be familiar with all technical details of OpenCRVS. Keep language high-level, user friendly and avoid technical references to internals.
  • Answer "What's new?", "Why was the change made?" and "Why should I care?" for each change.
  • If it's a breaking change, include a migration guide answering "What do I need to do to upgrade?".

@@ -258,6 +258,6 @@ export function findExtension<URL extends AllExtensions['url']>(
): Extract<AllExtensions, { url: URL }> | undefined {
return listOfExtensions.find(
(extension): extension is Extract<AllExtensions, { url: URL }> =>
extension.url === url
extension && extension.url === url
Copy link
Member

@rikukissa rikukissa Jan 3, 2025

Choose a reason for hiding this comment

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

Is there a specific valid case when the extension can be null? As per the type signature of this function, there really should be a case like that. If there is such a case, let's rather ensure null never gets into a list of extensions. Otherwise this will come back to haunt us when we migrate to Events v2 or do any other work with the potentially corrupted data.

@rikukissa
Copy link
Member

Root cause fixed in this PR:
#8299

@rikukissa rikukissa closed this Jan 8, 2025
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.

2 participants