Skip to content

fix(wrangler): skip non-successful deployments when tailing Pages logs#8847

Closed
mlafeldt wants to merge 2 commits intocloudflare:mainfrom
mlafeldt:fix-pages-tail
Closed

fix(wrangler): skip non-successful deployments when tailing Pages logs#8847
mlafeldt wants to merge 2 commits intocloudflare:mainfrom
mlafeldt:fix-pages-tail

Conversation

@mlafeldt
Copy link
Copy Markdown
Contributor

@mlafeldt mlafeldt commented Apr 8, 2025

Tailing logs of Pages deployments currently fails when a deployment was skipped, e.g. due to build watch paths.

$ wrangler pages deployment tail --project-name myproject
No deployment specified. Using latest deployment for production environment.

✘ [ERROR] A request to the Cloudflare API (/accounts/xxx/pages/projects/myproject/deployments/9dc87402-90c7-4227-b7cf-aa47630e588a/tails) failed.

  The requested Worker version could not be found, please check the ID being passed and try again.
  [code: 8000068]

Such skipped deployments have this latest stage:

latest_stage: { name: 'queued', started_on: null, ended_on: null, status: 'idle' },

(In an ideal world, the status should be skipped and not idle, but that's a different issue.)

To fix this problem, I changed the logic that finds the last deployment to consider successful deployments only.


  • Tests
    • TODO (before merge)
    • Tests included
    • Tests not necessary because:
  • Wrangler / Vite E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
    • I don't know
    • Required
    • Not required because:
  • Public documentation
    • TODO (before merge)
    • Cloudflare docs PR(s):
    • Documentation not necessary because:
  • Wrangler V3 Backport
    • TODO (before merge)
    • Wrangler PR:
    • Not necessary because:

@mlafeldt mlafeldt requested review from a team as code owners April 8, 2025 14:48
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 8, 2025

🦋 Changeset detected

Latest commit: beb6cf9

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

This PR includes changesets to release 2 packages
Name Type
wrangler Patch
@cloudflare/vitest-pool-workers 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

@mlafeldt mlafeldt force-pushed the fix-pages-tail branch 2 times, most recently from df31e3a to 5d57dfb Compare April 8, 2025 14:53
@github-project-automation github-project-automation Bot moved this to Untriaged in workers-sdk Apr 14, 2025
@petebacondarwin
Copy link
Copy Markdown
Contributor

@mlafeldt - thanks for this PR. Would you mind rebasing and then we can review and merge it.

@petebacondarwin petebacondarwin added the awaiting reporter response Needs clarification or followup from OP label May 19, 2025
@petebacondarwin petebacondarwin marked this pull request as draft May 19, 2025 13:45
@mlafeldt
Copy link
Copy Markdown
Contributor Author

@petebacondarwin In the meantime, I've figured that wrangler pages deployment tail uses the wrong (or at least suboptimal) API call. It should use canonical_deployment as returned by /accounts/{account_id}/pages/projects/{project_name} to get the most recent deployment. Do you agree? Happy to update the PR.

@petebacondarwin
Copy link
Copy Markdown
Contributor

Give it a go!

@mlafeldt mlafeldt marked this pull request as ready for review May 28, 2025 14:51
@mlafeldt
Copy link
Copy Markdown
Contributor Author

@petebacondarwin Rebased the PR. I ended up not using canonical_deployment because it only seems to exist for production and won't work with --environment preview.

Copy link
Copy Markdown
Contributor

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

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

@lrapoport-cf
Copy link
Copy Markdown
Contributor

hi @mlafeldt :) it looks like #9685 addresses the same problem as you are tackling here, but with one additional change to specify latest_stage.name to also be "deploy" to narrow down results to deployments only. as your PR also still needs tests, we will close this PR in favor of #9685. however, we recognize and really appreciate your contribution, so i've connected with @WillTaylorDev and he will add you as a coauthor on #9685 so that you receive proper acknowledgement.

thanks you for your contribution!

@github-project-automation github-project-automation Bot moved this from Untriaged to Done in workers-sdk Jun 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting reporter response Needs clarification or followup from OP

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants