trigger.dev icon indicating copy to clipboard operation
trigger.dev copied to clipboard

[TRI-1287] bug: dynamic trigger registrations that don't succeed should disable the event dispatcher

Open ericallam opened this issue 2 years ago • 1 comments

Provide environment information

  System:
    OS: macOS 13.5.1
    CPU: (10) arm64 Apple M1 Max
    Memory: 769.42 MB / 32.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 18.12.1 - ~/.nvm/versions/node/v18.12.1/bin/node
    npm: 8.19.2 - ~/.nvm/versions/node/v18.12.1/bin/npm
    pnpm: 7.18.1 - ~/.nvm/versions/node/v18.12.1/bin/pnpm

Describe the bug

When registering a dynamic trigger, if the registration job fails, the Event Dispatcher associated with that registration is still active. It needs to be disabled in the case the registration job fails. This could cause duplicate runs.

Reproduction repo

N/A

To reproduce

import { Github, events } from "@trigger.dev/github";

const dynamicOnIssueOpenedTrigger = client.defineDynamicTrigger({
  id: "github-issue-opened",
  event: events.onIssueOpened,
  source: github.sources.repo,
});

const github = new Github({ id: "github", token: process.env.GITHUB_PAT! });

client.defineJob({
  id: "register-issue-opened",
  name: "Register Issue Opened for Account",
  version: "0.0.1",
  trigger: eventTrigger({
    name: "register.issue.opened",
  }),
  run: async (payload, io, ctx) => {
    await dynamicOnIssueOpenedTrigger.register(
      payload.id,
      {
        owner: payload.owner,
        repo: payload.repo,
      }
    );
  },
});

client.defineJob({
  id: "dynamic-issue-opened",
  name: "Dynamic Issue Opened for Account",
  version: "0.0.1",
  trigger: dynamicOnIssueOpenedTrigger,
  integrations: {
    github,
  },
  run: async (payload, io, ctx) => {
    await io.github.issues.createComment("create-issue-comment", {
      owner: payload.repository.owner.login,
      repo: payload.repository.name,
      issueNumber: payload.issue.number,
      body: `Hello there: \n\n\`\`\`json\n${JSON.stringify(
        payload,
        null,
        2
      )}\`\`\`\n\n\`\`\`json\n${JSON.stringify(ctx, null, 2)}\`\`\``,
    });
  },
});

Use the preceding code and make sure the GITHUB_PAT does not have webhook read&write scope.

  1. Run the register-issue-opened job with the id, owner, and repo payload
  2. Fix the GITHUB_PAT to be able to do webhook read&write
  3. Make sure to restart
  4. Run the register-issue-opened job with the same id, owner, and repo payload as Step 1
  5. Create an issue on the repo
  6. Notice that two runs have occurred

Additional information

No response

TRI-1287

ericallam avatar Sep 21 '23 20:09 ericallam

@ericallam I wasn't able to reproduce this. The dynamic trigger re-registration reused the event dispatcher instead of creating a new one, so only one job was triggered.

Just a query to find the potential cause for this behaviour - Were the two runs observed in the dashboard or in the GitHub issue page? If it was the latter, there could be another job with the same trigger (open issue on the repo) and same run function that might have created the duplicate comment.

However, I did find an issue where the dynamic trigger re-registration was not working because the registration-id was reused as event-id for the worker event.

https://github.com/triggerdotdev/trigger.dev/blob/b9eba68078214c71716abe8ba9c39cf9f7149bb2/apps/webapp/app/routes/api.v2.%24endpointSlug.triggers.%24id.registrations.%24key.ts#L81-L88

IngestEventService will consider it an update-event flow, and default to a no-op (since the existing event is delivered already) - related issue #604. Before #604 went live, the service would have errored out and returned the existing row due to the eventId unique constraint in the DB (essentially not enqueuing the re-registration event). I've created #628 to fix this bug, please check.

hmacr avatar Oct 16 '23 11:10 hmacr