Skip to content

Update Agent.alarm to readonly, linking to schedule-task docs#260

Merged
threepointone merged 3 commits intocloudflare:mainfrom
nickfujita:alarm-readonly
May 7, 2025
Merged

Update Agent.alarm to readonly, linking to schedule-task docs#260
threepointone merged 3 commits intocloudflare:mainfrom
nickfujita:alarm-readonly

Conversation

@nickfujita
Copy link
Copy Markdown
Contributor

Since the Agent class has a custom implementation for scheduled tasks via this.schedule, if the alarm class is implemented on a subclass, it doesn't perform as expected. While a quick trip to the docs would point you in this direction, it would be nice to have a ts error and js doc explaining that instead of implementing alarm, use the schedule method instead. Was able to find a solution to provide a type warning in one case, but not sure if there is a better way to do this to catch all cases?

  • Updates alarm function on Agent class to be readonly, to produce a ts error on subclasses trying to override this method as an instance member function
  • Adds js doc to alarm function with link to schedule-task docs, in the hopes that this steers them in the right direction

Test happy path

  • In the AIChatAgent class, add a dummy alarm method
async alarm() {
  console.log('hello');
}
  • Produces the following type error
Class 'Agent<Env, State>' defines instance member property 'alarm', but extended class 'AIChatAgent<Env, State>' defines it as instance member function.ts(2425)
  • If the code is executed, the alarm function on AIChatAgent shouldn't be called, only the one on Agent

Possible subversion

If the user reads the error they could actually just get around this by instead adding to AIChatAgent

alarm = async () => {
  console.log('hello');
}

So it's not a fool proof solution, but the hope is that it would guard the majority that just follow the cloudflare docs on how to implement alarms.

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented May 7, 2025

🦋 Changeset detected

Latest commit: 71d1d27

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
agents Patch
hono-agents Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Copy Markdown
Contributor

@threepointone threepointone left a comment

Choose a reason for hiding this comment

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

lgtm, thank you!

@threepointone threepointone merged commit ca44ae8 into cloudflare:main May 7, 2025
1 check passed
@threepointone threepointone mentioned this pull request May 7, 2025
@nickfujita nickfujita deleted the alarm-readonly branch May 8, 2025 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants