🐛 Bug Report: Skipped inapp step will cause fail

[m1heng]

📜 Description

When previous inapp step is skipped, next step will show error.

 "statusCode": 400,
  "message": "Workflow with id: `test-workflow` has an invalid state. Step with id: `inapp` has invalid result. Please provide the correct step result.",
  "code": "ExecutionStateResultInvalidError",
  "data": [
    {
      "path": "",
      "message": "must have required property 'seen'"
    }

👟 Reproduction steps

const testWorkflow = workflow('test-workflow', async ({ step, payload }) => {
    await step.inApp('inapp', async () => {
        return {
            body: 'This is a log message',
        }
    }, {
        skip: () => payload.userName === 'John Doe',
    });

    await step.email('send-email', async (controls) => {
        return {
            subject: controls.subject,
            body: 'This is your first Novu Email ' + payload.userName,
        };
    },
        {
            controlSchema: z.object({
                subject: z.string().default('A Successful Test on Novu from {{userName}}'),
            }),
        });
}, {
    payloadSchema: z.object({
        userName: z.string().default('John Doe'),
    }),
});

const app = express();
app.use(express.json()); // Required for Novu POST requests
app.use("/api/novu", serve({ workflows: [testWorkflow] }));

app.listen(3000, () => {
    console.log("Server running on port 3000");
});

👍 Expected behavior

next step should run without error

👎 Actual Behavior with Screenshots

Novu version

Novu SaaS

npm version

No response

node version

No response

📃 Provide any additional context for the Bug.

No response

👀 Have you spent some time to check if this bug has been raised before?

• I checked and didn't find a similar issue

🏢 Have you read the Contributing Guidelines?

• I have read the Contributing Guidelines

Are you willing to submit PR?

Yes I am willing to submit a PR!

[linear]

NV-5120 🐛 Bug Report: Skipped inapp step will cause fail

[m1heng]

Also happens when previews step is a custom step with required output schema
Code

const testWorkflow = workflow('test-workflow', async ({ step, payload }) => {
    const resp = await step.custom('custom', async () => {
        return {
            body: 'This is a log message',
            subject: "dsad"
        }
    }, {
        skip: () => payload.userName === 'John Doe',
        outputSchema: z.object({
            body: z.string(),
            subject: z.string(),
        }),
    });


    await step.email('send-email', async (controls) => {
        return {
            subject: controls.subject,
            body: 'This is your first Novu Email ' + payload.userName,
        };
    },
        {
            controlSchema: z.object({
                subject: z.string().default('A Successful Test on Novu from {{userName}}'),
            }),
        });
}, {
    payloadSchema: z.object({
        userName: z.string().default('John Doe'),
    }),
});

Error

{
  "message": "Workflow with id: `test-workflow` has an invalid state. Step with id: `custom` has invalid result. Please provide the correct step result.",
  "code": "ExecutionStateResultInvalidError",
  "data": [
    {
      "path": "/body",
      "message": "Required"
    },
    {
      "path": "/subject",
      "message": "Required"
    }
}

Root reason seems to be in https://github.com/m1heng/novu/blob/fix/api-property/packages/framework/src/client.ts#L314-L333
In executeStepFactory, skip only happens when real execution happens but not during hydration of preview step, so when hydrating a skipped step, it will goes into a validation of result and output which will cause above errors.

[m1heng]

A simple way to deal with it is add skip check for non current step, but this would leads to a problem that whether should the framework ask developer to make their skip function to be idempotent during runs.
If not, we should let the bridge know that a step was skipped, which means the payload towards bridge should contain that information. As far as I discovered, it can be done in https://github.com/m1heng/novu/blob/fix/api-property/apps/worker/src/app/workflow/usecases/execute-bridge-job/execute-bridge-job.usecase.ts#L171, where we can add job status (which contain a status named SKIPPED) as a property in mapState return.
But here is the thing that I can not find where will it mark the job as SKIPPED as there is not reference of JobStatusEnum.SKIPPED.
In https://github.com/m1heng/novu/blob/fix/api-property/apps/worker/src/app/workflow/usecases/send-message/send-message.usecase.ts#L111, it will mark job as CANCELED when isBridgeSkipped is true.
So more discussion is needed from the team.