feat(functions): Add features to task queue functions #2216
Conversation
| @@ -58,6 +60,38 @@ export type TaskOptions = DeliverySchedule & TaskOptionsExperimental & { | |||
| * The default is 10 minutes. The deadline must be in the range of 15 seconds and 30 minutes. | |||
| */ | |||
| dispatchDeadlineSeconds?: number; | |||
| /** | |||
There was a problem hiding this comment.
nit: newline between commented fields (e.g. here and headers)
| }); | ||
| } | ||
|
|
||
| for (const invalidOption of [null, 'abc', '', [], true, 102, 1.2]) { | ||
| it(`should throw if options is ${invalidOption}`, () => { | ||
| expect(() => apiClient.enqueue({}, FUNCTION_NAME, '', invalidOption as any)) | ||
| .to.throw('TaskOptions must be a non-null object'); | ||
| expect(apiClient.enqueue({}, FUNCTION_NAME, '', invalidOption as any)) |
There was a problem hiding this comment.
Here and elsewhere, you need to make the test case async and then await this expectation or no check will actually be performed
There was a problem hiding this comment.
Hm I deliberately chose to use chai's eventually to wait on the Promise returned by enqueue calls instead of async/await, to align better with the existing implementation which generally avoids async/await. Should I go ahead and refactor the unit tests to use async/await instead?
package-lock.json
Outdated
There was a problem hiding this comment.
Since we don't make any changes to the package.json in this PR, let's revert the lock file changes and let dependabot take care of the package-lock.json updates.
lahirumaramba
changed the title
src/functions/functions-api.ts
Outdated
| * If not provided, one will be automatically generated. | ||
| * If provided explicitly specifying a task ID enables task de-duplication. If a task's ID is | ||
| * identical to that of an existing task or a task that was deleted or executed recently then | ||
| * the call will throw a TaskAlreadyExists error. Another task with the same id can't be |
There was a problem hiding this comment.
Suggest TaskAlreadyExists and capped "ID" in the next line.
There was a problem hiding this comment.
Actually wanted to get your thoughts on how to document this — instead of throwing a TaskAlreadyExists error, we changed the behavior to throw a FirebaseFunctionsError with code "functions/task-already-exists". So if a user wanted to check if a thrown error is due to the task already existing, they would check like so:
try {
await queue.enqueue(...);
} catch (err: unknown) {
if (err.code === "functions/task-already-exists") {
...
}
}
Is this comment in the functions API sufficient to document this behavior?
Since CF3 released support for Task Queue Functions at Google I/O 2022, users have made several feature requests. This PR enhances the Task Queue functions API to enable the following:
idto TaskOptions when enqueueing tasks.idto the newdeleteAPI.See firebase/firebase-functions#1423 for the sister changes in the functions SDK.
RELEASE NOTES: Headers passed to TaskQueue HTTPS handlers are now available in the context/event. Added the ability to name the tasks by including an
idwhen enqueueing tasks. Added the ability to delete an enqueued task if it has not yet completed.