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

[TRI-950] io.runTask return type of the callback has TypeScript error when it includes an unknown type #280

Closed
ericallam opened this issue Aug 8, 2023 · 19 comments
Labels
area/server Issues related to the Trigger.dev server 💎 Bounty bug Something isn't working 💰 Rewarded
Milestone

Comments

@ericallam
Copy link
Member

ericallam commented Aug 8, 2023

To recreate this issue:

await io.runTask("foo", { name: "Foo" }, async () => {
  return {
    foo: "bar",
  } as unknown;
});

That is because the return type of the callback is typed as required to extend SerializedJson:

async runTask<TResult extends SerializableJson | void = void>(

We do this so the types of the return of runTask will match what is actually returned, since we run the results of the runTask callback through a JSON stringify/parsing as task output is stored in the database as JSON. Because of Resumability, this is a requirement, unless another solution can be identified.

I'm also now realizing that this SerializedJson might be incorrectly typed as well, as it can include Date and Symbol:

const SerializableSchema = z.union([

So this could cause issues when resuming a run and using the cached task output, where Date's would be symbols and the types would be wrong. It would be great to test this and possible change the required type to DeserializedJson.

TRI-950

@maige-app maige-app bot added bug Something isn't working area/server Issues related to the Trigger.dev server labels Aug 8, 2023
@ericallam ericallam changed the title io.runTask return type of the callback has TypeScript error when it includes an unknown type [TRI-950] io.runTask return type of the callback has TypeScript error when it includes an unknown type Aug 8, 2023
@ericallam
Copy link
Member Author

/bounty $75

@algora-pbc
Copy link

algora-pbc bot commented Aug 8, 2023

💎 $75 bounty created by ericallam
🙋 If you'd like to work on this issue, comment below to get assigned
👉 To claim this bounty, submit a pull request that includes the text /claim #280 somewhere in its body
📝 Before proceeding, please make sure you can receive payouts in your country
💵 Payment arrives in your account 2-5 days after the bounty is rewarded
💯 You keep 100% of the bounty award
🙏 Thank you for contributing to triggerdotdev/trigger.dev!

@Akshay-Patel-dev
Copy link
Contributor

Akshay-Patel-dev commented Aug 8, 2023

/attempt #280

Options

@MeenuyD
Copy link

MeenuyD commented Aug 8, 2023

/attempt #280

Options

@algora-pbc
Copy link

algora-pbc bot commented Aug 8, 2023

@Akshay-Patel-dev is already attempting to complete #280 and claim the bounty. If you attempt to complete the same issue, there is a chance that @Akshay-Patel-dev will complete the issue first, and be awarded the bounty. Try discussing with @Akshay-Patel-dev and potentially collaborating on the same solution versus creating an alternate solution.

@MeenuyD
Copy link

MeenuyD commented Aug 9, 2023

Hello,@ericallam Can you explain a bit more

@ologbonowiwi
Copy link
Contributor

ologbonowiwi commented Aug 12, 2023

I was able to reproduce the issue but it is not clear what needs to be done so far. It would be nice to get some details

/attempt #280

Options

@yashug
Copy link
Contributor

yashug commented Aug 12, 2023

how about we having a wrapper something like this

async function runTaskWrapper<TResult>(
  key: string | any[],
  options: RunTaskOptions,
  callback: (task: IOTask, io: IO) => Promise<TResult>
): Promise<SerializableJson> {
  const result = await io.runTask(key, options, async (task, io) => {
    const userResult = await callback(task, io);
    // Ensure that userResult is of SerializableJson type
    return userResult as SerializableJson;
  });

  return result;
}

@ologbonowiwi
Copy link
Contributor

@yashug even thought this would work, it's not safety right? We would be getting a type safety at transpile/development, but on runtime the data type could be different and bring unexpected behaviors

@algora-pbc
Copy link

algora-pbc bot commented Aug 15, 2023

@Akshay-Patel-dev: Reminder that in 7 days the bounty will become up for grabs, so please submit a pull request before then 🙏

@ologbonowiwi
Copy link
Contributor

@ericallam, can you take a look at #342?

I decided to take a shot, and if you test with your example

await io.runTask("foo", { name: "Foo" }, async () => {
  return {
    foo: "bar",
  } as unknown;
});

It's working now

@ologbonowiwi
Copy link
Contributor

So this could cause issues when resuming a run and using the cached task output, where Date's would be symbols and the types would be wrong. It would be great to test this and possible change the required type to DeserializedJson.

Do you see a way of handling this without a breaking change?

@algora-pbc
Copy link

algora-pbc bot commented Aug 15, 2023

💡 @ologbonowiwi submitted a pull request that claims the bounty. You can visit your org dashboard to reward.
👉 @ologbonowiwi: To receive payouts, sign up on Algora, link your Github account and connect with Stripe on your dashboard.

@algora-pbc
Copy link

algora-pbc bot commented Aug 15, 2023

@MeenuyD: Reminder that in 7 days the bounty will become up for grabs, so please submit a pull request before then 🙏

@algora-pbc
Copy link

algora-pbc bot commented Aug 18, 2023

@ologbonowiwi: Your claim has been rewarded! 👉 Complete your Algora onboarding to collect the bounty.

@algora-pbc
Copy link

algora-pbc bot commented Aug 19, 2023

🎉🎈 @ologbonowiwi has been awarded $75! 🎈🎊

@ologbonowiwi
Copy link
Contributor

Can we close this one guys? @ericallam @matt-aitken

@ologbonowiwi
Copy link
Contributor

Can we close it, guys? @matt-aitken @ericallam

@matt-aitken
Copy link
Member

Yep @ologbonowiwi, I'll close now

@ericallam ericallam added this to the v.15 milestone Oct 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/server Issues related to the Trigger.dev server 💎 Bounty bug Something isn't working 💰 Rewarded
Projects
None yet
Development

No branches or pull requests

6 participants