[go: nahoru, domu]
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.

Sign up for GitHub

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

Jump to bottom

Feature request: Pass retry count to task queue function #1178

Closed
gustavopch opened this issue Jul 25, 2022 · 9 comments
Closed

Feature request: Pass retry count to task queue function #1178

gustavopch opened this issue Jul 25, 2022 · 9 comments
Labels
type: feature request

Comments

@gustavopch
Copy link
gustavopch commented Jul 25, 2022 •
edited
Loading

I've tweeted about this and @mbleigh instructed me to file a feature request in the repo, so:

I know that Cloud Tasks sends the HTTP request with a header X-CloudTasks-TaskRetryCount. I need to have that information in the Firebase task queue function so I can handle the last attempt differently, e.g.:

  • Register somewhere that the task was left in a failed state.
  • Send an email to the user informing about the failure and asking to retry manually later.
  • Increase some failure counter.

Maybe consider exposing the raw request just as we can access context.rawRequest in the callable functions.

As @inlined said in this tweet, he is not a fan of exposing the raw request because it would be harder to test. On the other hand, maybe you can expose the retry count as a first-class parameter like context.retryCount, but still expose context.rawRequest as an escape hatch to be used while a first-class parameter doesn't exist. For example, right now I'd be able to use context.rawRequest.headers.get('x-cloudtasks-taskretrycount') until a better parameter is implemented.

If such an escape hatch doesn't exist, people like me either have to wait until the team expose the desired parameter or have to rewrite to use the Cloud Tasks client directly. Using context.rawRequest temporarily would be a much easier workaround even if it's not documented or easy to test.
@google-oss-bot
Copy link
Collaborator

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

@mbleigh
Copy link
Contributor
mbleigh commented Jul 25, 2022

For reference, we may want to expose any of these that seem useful: https://cloud.google.com/tasks/docs/creating-http-target-tasks#handler

@gustavopch
Copy link
Author

To know if an attempt is the last attempt, we must also know how maxAttempts changes on redeploy.

For example, suppose I have a function with maxAttempts: 5 and the first attempt failed. Before it's retried, I redeploy the function with maxAttempts: 1. Will the failed task respect the new config and not be retried at all? What if I change to maxAttempts: 20, will it now possibly retry 19 more times or will it still respect the original config of maxAttempts: 5 and retry at most 4 more times?

If tasks always follow the most recent maxAttempts config, then maybe it's fine to have it hard coded in the function. Otherwise, maxAttempts has to be dynamically injected.

@inlined
Copy link
Member
inlined commented Aug 9, 2022

This feature has been formally proposed internally.

@taeold taeold mentioned this issue Jan 19, 2023
Open
@Juici
Copy link
Juici commented Feb 8, 2023

Any update on this after 5 months? It's a bit of a joke that I have to maintain a fork of the package just to have this feature.

@gustavopch
Copy link
Author

I'm a bit tired of waiting Firebase to implement new things. Everything takes ages. I used to be excited by the Firebase Summit, but it's getting ever more boring year after year. Note that I'm not blaming the team, it's probably just lack of investment from Google, but it's sad anyway.

@sdegroot
Copy link

I'd really love to see this feature as well. Without the extra context that is normally provided in the headers, one really has limited control over the tasks. It is impossible now to determine if the tasks will be retried again or if we have to move it to some dead letter storage.

Is there anything we can do to help @inlined ?

@marvin-kolja
Copy link

@inlined I might be wrong but isn't this already implemented by #1423? Also #1340 should be closed since it's requesting a similar feature.

@gustavopch
Copy link
Author

@marvin-kolja You're right. I'm closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
type: feature request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@mbleigh @sdegroot @inlined @gustavopch @taeold @Juici @google-oss-bot @marvin-kolja