qfix: Fix github measurements (#9816)

1. Fix github measurements 2. A bit more proper fix for caching of Octokit references Signed-off-by: Andrey Sobolev <haiodo@gmail.com>
2026-08-25 22:02:23 +02:00 · 2025-09-10 00:00:08 +05:00
parent eb9150541b
commit 04a9c52d48
12 changed files with 238 additions and 124 deletions
@@ -80,8 +80,14 @@ export class CommentSyncManager implements DocSyncManager {
    await this.eventSync.get(event.issue.url)
    const promise = this.processEvent(ctx, event, derivedClient, integration)
    this.eventSync.set(event.issue.url, promise)
-    await promise
-    this.eventSync.delete(event.issue.url)
+    try {
+      await promise
+      this.eventSync.delete(event.issue.url)
+    } catch (err: any) {
+      ctx.error('Error processing event', { error: err })
+    } finally {
+      this.eventSync.delete(event.issue.url)
+    }
  }

  async handleDelete (
@@ -22,7 +22,8 @@ import core, {
  Ref,
  Space,
  TxOperations,
-  makeDocCollabId
+  makeDocCollabId,
+  withContext
 } from '@hcengineering/core'
 import github, { DocSyncInfo, GithubIntegrationRepository, GithubIssue, GithubProject } from '@hcengineering/github'
 import { IntlString } from '@hcengineering/platform'
@@ -112,6 +113,7 @@ export abstract class IssueSyncManagerBase {
    return socialIds.map((it) => it.attachedTo)
  }

+  @withContext('issues-handleUpdate')
  async handleUpdate (
    ctx: MeasureContext,
    external: IssueExternalData,
@@ -135,12 +137,16 @@ export abstract class IssueSyncManagerBase {

    syncData =
      syncData ??
-      (await this.client.findOne(github.class.DocSyncInfo, { space: prj._id, url: (external.url ?? '').toLowerCase() }))
+      (await ctx.with('findDocInfo', {}, (ctx) =>
+        this.client.findOne(github.class.DocSyncInfo, { space: prj._id, url: (external.url ?? '').toLowerCase() })
+      ))

    if (syncData !== undefined) {
-      const doc: Issue | undefined = await this.client.findOne<Issue>(syncData.objectClass, {
-        _id: syncData._id as unknown as Ref<Issue>
-      })
+      const doc: Issue | undefined = await ctx.with('find pull request', {}, (ctx) =>
+        this.client.findOne<Issue>(syncData.objectClass, {
+          _id: syncData._id as unknown as Ref<Issue>
+        })
+      )

      // Use now as modified date for events.
      const lastModified = new Date().getTime()
@@ -153,6 +159,7 @@ export abstract class IssueSyncManagerBase {
        ) {
          try {
            const collabId = makeDocCollabId(doc, 'description')
+
            await this.collaborator.updateMarkup(collabId, update.description)
          } catch (err: any) {
            Analytics.handleError(err)
@@ -184,35 +191,39 @@ export abstract class IssueSyncManagerBase {
          await updateTodos.commit()
        }

-        await derivedClient.diffUpdate(
-          syncData,
-          {
-            external,
-            externalVersion: githubExternalSyncVersion,
-            current: { ...syncData.current, ...update },
-            needSync: needSync ? '' : githubSyncVersion, // No need to sync after operation.
-            derivedVersion: '', // Check derived changes
-            lastModified,
-            lastGithubUser: account,
-            ...extraSyncUpdate
-          },
-          lastModified
+        await ctx.with('diffUpdate syncData', {}, (ctx) =>
+          derivedClient.diffUpdate(
+            syncData,
+            {
+              external,
+              externalVersion: githubExternalSyncVersion,
+              current: { ...syncData.current, ...update },
+              needSync: needSync ? '' : githubSyncVersion, // No need to sync after operation.
+              derivedVersion: '', // Check derived changes
+              lastModified,
+              lastGithubUser: account,
+              ...extraSyncUpdate
+            },
+            lastModified
+          )
        )
-        await this.client.diffUpdate(doc, issueData, lastModified, account)
+        await ctx.with('diffUpdate-issue', {}, (ctx) => this.client.diffUpdate(doc, issueData, lastModified, account))
        this.provider.sync()
      } else if (doc === undefined) {
-        await derivedClient.diffUpdate(
-          syncData,
-          {
-            external,
-            externalVersion: githubExternalSyncVersion,
-            needSync: '',
-            derivedVersion: '', // Check derived changes
-            lastModified,
-            lastGithubUser: account,
-            ...extraSyncUpdate
-          },
-          lastModified
+        await ctx.with('diffUpdate-syncData', {}, (ctx) =>
+          derivedClient.diffUpdate(
+            syncData,
+            {
+              external,
+              externalVersion: githubExternalSyncVersion,
+              needSync: '',
+              derivedVersion: '', // Check derived changes
+              lastModified,
+              lastGithubUser: account,
+              ...extraSyncUpdate
+            },
+            lastModified
+          )
        )
      }
    }
@@ -267,6 +278,7 @@ export abstract class IssueSyncManagerBase {
    info: DocSyncInfo
  ): Promise<void>

+  @withContext('issues-handleDiffUpdate')
  async handleDiffUpdate (
    ctx: MeasureContext,
    container: ContainerFocus,
@@ -282,7 +294,7 @@ export abstract class IssueSyncManagerBase {
      await ctx.with(
        'create mixin issue: GithubIssue',
        {},
-        async () => {
+        async (ctx) => {
          await this.client.createMixin<Issue, GithubIssue>(
            existing._id as Ref<GithubIssue>,
            existing._class,
@@ -111,11 +111,16 @@ export class IssueSyncManager extends IssueSyncManagerBase implements DocSyncMan
      const urlId = issueEvent.issue.url

      await syncRunner.exec(urlId, async () => {
-        await this.processEvent(ctx, issueEvent, derivedClient, repository, integration, project)
+        try {
+          await this.processEvent(ctx, issueEvent, derivedClient, repository, integration, project)
+        } catch (err: any) {
+          ctx.error('Error processing event', { error: err })
+        }
      })
    }
  }

+  @withContext('issues-processEvent')
  private async processEvent (
    ctx: MeasureContext,
    event: IssuesEvent,
@@ -129,8 +134,9 @@ export class IssueSyncManager extends IssueSyncManagerBase implements DocSyncMan
    let externalData: IssueExternalData | undefined
    if (event.action !== 'deleted') {
      try {
-        const response: any = await integration.octokit?.graphql(
-          `query listIssue($name: String!, $owner: String!, $issue: Int!) {
+        const response: any = await ctx.with('graphql', {}, (ctx) =>
+          integration.octokit?.graphql(
+            `query listIssue($name: String!, $owner: String!, $issue: Int!) {
          repository(name: $name, owner: $owner) {
            issue(number: $issue) {
              ${issueDetails(true)}
@@ -138,11 +144,12 @@ export class IssueSyncManager extends IssueSyncManagerBase implements DocSyncMan
          }
        }
        `,
-          {
-            name: repo.name,
-            owner: repo.owner?.login,
-            issue: event.issue.number
-          }
+            {
+              name: repo.name,
+              owner: repo.owner?.login,
+              issue: event.issue.number
+            }
+          )
        )
        externalData = response.repository.issue
      } catch (err: any) {
@@ -351,7 +358,7 @@ export class IssueSyncManager extends IssueSyncManagerBase implements DocSyncMan
      const description = await ctx.with(
        'query collaborative description',
        {},
-        async () => {
+        async (ctx) => {
          const collabId = makeDocCollabId(existing, 'description')
          return await this.collaborator.getMarkup(collabId, (existing as Issue).description)
        },
@@ -367,7 +374,7 @@ export class IssueSyncManager extends IssueSyncManagerBase implements DocSyncMan
      const createdIssueData = await ctx.with(
        'create github issue',
        {},
-        async () => {
+        async (ctx) => {
          this.createPromise = this.createGithubIssue(
            ctx,
            container,
@@ -501,10 +508,11 @@ export class IssueSyncManager extends IssueSyncManagerBase implements DocSyncMan
        await ctx.with(
          'create platform issue',
          {},
-          async () => {
+          async (ctx) => {
            const st = (await guessStatus(issueExternal, statuses))._id as Ref<Status>

            await this.createNewIssue(
+              ctx,
              info,
              accountGH,
              {
@@ -555,7 +563,7 @@ export class IssueSyncManager extends IssueSyncManagerBase implements DocSyncMan
        const updateResult = await ctx.with(
          'diff update',
          {},
-          async () =>
+          async (ctx) =>
            await this.handleDiffUpdate(
              ctx,
              container,
@@ -655,7 +663,7 @@ export class IssueSyncManager extends IssueSyncManagerBase implements DocSyncMan
        await ctx.with(
          '==> updateIssue',
          {},
-          async () => {
+          async (ctx) => {
            ctx.info('update fields', {
              url: issueExternal.url,
              ...issueUpdate,
@@ -696,7 +704,7 @@ export class IssueSyncManager extends IssueSyncManagerBase implements DocSyncMan
        await ctx.with(
          '==> updateIssue',
          {},
-          async () => {
+          async (ctx) => {
            ctx.info('update fields', { ...issueUpdate, workspace: this.provider.getWorkspaceId() })
            if (isGHWriteAllowed()) {
              const hasOtherChanges = Object.keys(issueUpdate).length > 0
@@ -807,7 +815,9 @@ export class IssueSyncManager extends IssueSyncManagerBase implements DocSyncMan
    }
  }

+  @withContext('issues-createNewIssue')
  private async createNewIssue (
+    ctx: MeasureContext,
    info: DocSyncInfo,
    account: PersonId,
    issueData: GithubIssueData & { status: Issue['status'] },
@@ -930,7 +940,7 @@ export class IssueSyncManager extends IssueSyncManagerBase implements DocSyncMan
          const response: any = await ctx.with(
            'graphql.listIssue',
            {},
-            () =>
+            (ctx) =>
              integration.octokit.graphql(
                `query listIssues {
                    nodes(ids: [${idsp}] ) {
@@ -122,11 +122,16 @@ export class PullRequestSyncManager extends IssueSyncManagerBase implements DocS
      const url = event.pull_request.issue_url

      await syncRunner.exec(url, async () => {
-        await this.processEvent(ctx, event, derivedClient, repository, integration, project)
+        try {
+          await this.processEvent(ctx, event, derivedClient, repository, integration, project)
+        } catch (err: any) {
+          ctx.error('Error processing event', { error: err })
+        }
      })
    }
  }

+  @withContext('pullrequests-processEvent')
  private async processEvent (
    ctx: MeasureContext,
    event: PullRequestEvent,
@@ -139,8 +144,9 @@ export class PullRequestSyncManager extends IssueSyncManagerBase implements DocS

    let externalData: PullRequestExternalData
    try {
-      const response: any = await integration.octokit?.graphql(
-        `query listIssue($name: String!, $owner: String!, $issue: Int!) {
+      const response: any = await ctx.with('graphql', {}, (ctx) =>
+        integration.octokit?.graphql(
+          `query listIssue($name: String!, $owner: String!, $issue: Int!) {
          repository(name: $name, owner: $owner) {
            pullRequest(number: $issue) {
              ${pullRequestDetails}
@@ -148,11 +154,12 @@ export class PullRequestSyncManager extends IssueSyncManagerBase implements DocS
          }
        }
        `,
-        {
-          name: repo.name,
-          owner: repo.owner?.login,
-          issue: event.pull_request.number
-        }
+          {
+            name: repo.name,
+            owner: repo.owner?.login,
+            issue: event.pull_request.number
+          }
+        )
      )
      externalData = response.repository.pullRequest
    } catch (err: any) {
@@ -496,7 +503,7 @@ export class PullRequestSyncManager extends IssueSyncManagerBase implements DocS
        }

        // To sync reviews/review threads in case they are created before us.
-        await syncChilds(info, this.client, derivedClient)
+        await syncChilds(ctx, info, this.client, derivedClient)

        return {
          needSync: '',
@@ -88,8 +88,13 @@ export class ReviewCommentSyncManager implements DocSyncManager {
    await this.eventSync.get(event.comment.html_url)
    const promise = this.processEvent(ctx, event, derivedClient, repository, integration)
    this.eventSync.set(event.comment.html_url, promise)
-    await promise
-    this.eventSync.delete(event.comment.html_url)
+    try {
+      await promise
+    } catch (err: any) {
+      ctx.error('Error processing event', { error: err })
+    } finally {
+      this.eventSync.delete(event.comment.html_url)
+    }
  }

  async handleDelete (
@@ -364,7 +369,7 @@ export class ReviewCommentSyncManager implements DocSyncManager {
    }
    if (existing === undefined) {
      try {
-        await this.createReviewComment(info, messageData, parent, reviewComment, account)
+        await this.createReviewComment(ctx, info, messageData, parent, reviewComment, account)
        return { needSync: githubSyncVersion, current: messageData }
      } catch (err: any) {
        ctx.error('Error', { err })
@@ -387,6 +392,7 @@ export class ReviewCommentSyncManager implements DocSyncManager {
    return { current: messageData, needSync: githubSyncVersion }
  }

+  @withContext('handleDiffUpdate-comment')
  private async handleDiffUpdate (
    ctx: MeasureContext,
    existing: Doc,
@@ -458,7 +464,9 @@ export class ReviewCommentSyncManager implements DocSyncManager {
    }
  }

+  @withContext('review-comments-createReviewComment')
  private async createReviewComment (
+    ctx: MeasureContext,
    info: DocSyncInfo,
    messageData: ReviewCommentData,
    parent: DocSyncInfo,
@@ -482,6 +490,7 @@ export class ReviewCommentSyncManager implements DocSyncManager {
    )
  }

+  @withContext('review-comments-create')
  async createGithubReviewComment (
    ctx: MeasureContext,
    container: ContainerFocus,
@@ -108,8 +108,13 @@ export class ReviewThreadSyncManager implements DocSyncManager {
    await this.eventSync.get(event.thread.node_id)
    const promise = this.processEvent(ctx, event, derivedClient, repository, integration)
    this.eventSync.set(event.thread.node_id, promise)
-    await promise
-    this.eventSync.delete(event.thread.node_id)
+    try {
+      await promise
+    } catch (err: any) {
+      ctx.error('Error processing event', { error: err })
+    } finally {
+      this.eventSync.delete(event.thread.node_id)
+    }
  }

  async handleDelete (
@@ -311,7 +316,7 @@ export class ReviewThreadSyncManager implements DocSyncManager {
        await this.createReviewThread(info, messageData, parent, review, account)

        // We need trigger comments, if their sync data created before
-        await syncChilds(info, this.client, derivedClient)
+        await syncChilds(ctx, info, this.client, derivedClient)
        return { needSync: githubSyncVersion, current: messageData }
      } catch (err: any) {
        ctx.error('Error', { err })
@@ -86,8 +86,13 @@ export class ReviewSyncManager implements DocSyncManager {
    await this.eventSync.get(event.review.html_url)
    const promise = this.processEvent(ctx, event, derivedClient, repository, integration)
    this.eventSync.set(event.review.html_url, promise)
-    await promise
-    this.eventSync.delete(event.review.html_url)
+    try {
+      await promise
+    } catch (err: any) {
+      ctx.error('Error processing event', { error: err })
+    } finally {
+      this.eventSync.delete(event.review.html_url)
+    }
  }

  async handleDelete (
@@ -318,9 +323,9 @@ export class ReviewSyncManager implements DocSyncManager {
    }
    if (existing === undefined) {
      try {
-        await this.createReview(info, messageData, parent, review, account)
+        await this.createReview(ctx, info, messageData, parent, review, account)

-        await syncChilds(info, this.client, derivedClient)
+        await syncChilds(ctx, info, this.client, derivedClient)
        return { needSync: githubSyncVersion, current: messageData }
      } catch (err: any) {
        ctx.error('Error', { err })
@@ -328,12 +333,14 @@ export class ReviewSyncManager implements DocSyncManager {
        return { needSync: githubSyncVersion, error: errorToObj(err) }
      }
    } else {
-      await this.handleDiffUpdate(existing, info, messageData, container, parent, review, account)
+      await this.handleDiffUpdate(ctx, existing, info, messageData, container, parent, review, account)
    }
    return { current: messageData, needSync: githubSyncVersion }
  }

+  @withContext('reviews-handleDiffUpdate')
  private async handleDiffUpdate (
+    ctx: MeasureContext,
    existing: Doc,
    info: DocSyncInfo,
    reviewData: ReviewData,
@@ -379,7 +386,9 @@ export class ReviewSyncManager implements DocSyncManager {
    }
  }

+  @withContext('reviews-createReview')
  private async createReview (
+    ctx: MeasureContext,
    info: DocSyncInfo,
    messageData: ReviewData,
    parent: DocSyncInfo,
@@ -403,6 +412,7 @@ export class ReviewSyncManager implements DocSyncManager {
    )
  }

+  @withContext('reviews-createGithubReview')
  async createGithubReview (
    ctx: MeasureContext,
    container: ContainerFocus,
@@ -196,9 +196,11 @@ export class SyncRunner {
      id,
      promise.then(() => {})
    )
-    const result = await promise
-    this.eventSync.delete(id)
-    return result
+    try {
+      return await promise
+    } finally {
+      this.eventSync.delete(id)
+    }
  }
 }

@@ -328,13 +330,20 @@ export function compareMarkdown (a: string, b: string): boolean {
  return na === nb
 }

-export async function syncChilds (info: DocSyncInfo, client: TxOperations, derivedClient: TxOperations): Promise<void> {
-  const childInfos = await client.findAll(github.class.DocSyncInfo, { parent: info.url.toLowerCase() })
+export async function syncChilds (
+  ctx: MeasureContext,
+  info: DocSyncInfo,
+  client: TxOperations,
+  derivedClient: TxOperations
+): Promise<void> {
+  const childInfos = await ctx.with('syncChilds-find', {}, () =>
+    client.findAll(github.class.DocSyncInfo, { parent: info.url.toLowerCase() })
+  )
  if (childInfos.length > 0) {
    const ops = derivedClient.apply()
    for (const child of childInfos) {
      await ops?.update(child, { needSync: '' })
    }
-    await ops.commit()
+    await ctx.with('sync-child-trigger', {}, () => ops.commit())
  }
 }