cloudflare / workers-sdk

โ›…๏ธ Home to Wrangler, the CLI for Cloudflare Workersยฎ
https://developers.cloudflare.com/workers/
Apache License 2.0
2.71k stars 712 forks source link

๐Ÿ› BUG: No error handling for failed deploy due to binding with the name 'assets'. #6266

Closed db-cloudflare closed 2 weeks ago

db-cloudflare commented 4 months ago

Which Cloudflare product(s) does this pertain to?

Pages

What version(s) of the tool(s) are you using?

Wrangler 3.64.0

What version of Node are you using?

20.10.0

What operating system and version are you using?

Windows 11

Describe the Bug

Observed behavior

When deploying a Hono application in Wrangler, if a binding uses the name 'ASSETS' the application will fail to deploy.

Expected behavior

The application failing to deploy is expected as assets is a reserved keyword, however no error message is provided to explain that this is the cause of the failed deployment. An error message that explains that a binding with the name assets exists and that the name needs to be changed is the expected behavior.

Steps to reproduce

Please provide the following:

petebacondarwin commented 4 months ago

I cannot reproduce this. I followed the following steps:

  1. Create new Hono application via pnpm create-cloudflare@latest
  2. Add an ASSETS binding in wrangler.toml:
    [vars]
    ASSETS = "production_value"
  3. Use the binding in the main handler:
    app.get("/", (c) => {
    return c.text("Hello Hono!" + c.env.ASSETS);
  4. Deploy the app

Working at https://holy-block-8fb2.petebd.workers.dev

db-cloudflare commented 4 months ago

Apologies, I should have been more specific with the type of binding involved. In this case, it was an R2 binding with the binding name as ASSETS like the following that resulted in an error when deploying:

[[r2_buckets]] binding = "ASSETS" bucket-name = "static-assets"

threepointone commented 4 months ago

I think this is a problem specifically with pages, is that right @dbenCF?

petebacondarwin commented 4 months ago

Oh right. Hono created via C3 is for Workers. But yes, I expect that ASSETS is blocked in Cloudflare Pages. Let me try to recreate it...

petebacondarwin commented 4 months ago

OK I managed to recreate this, I think. I don't think this has anything to do with Hono or R2. I think that any Pages project that is configured with a wrangler.toml that has an ASSETS binding (of any kind) will error with an output that looks like:

pnpm wrangler pages deploy 
๐ŸŒ  Uploading... (1/1)

โœจ Success! Uploaded 0 files (1 already uploaded) (0.16 sec)

โœจ Compiled Worker successfully
โœจ Uploading Worker bundle
๐ŸŒŽ Deploying...

โœ˜ [ERROR] Deployment failed!

  Failed to publish your Function. Got error: Unknown internal error occurred.
petebacondarwin commented 4 months ago

Indeed I can recreate this with the following files:

_worker.js

export default {
  fetch(_req, env) {
    return new Response(env.ASSETS);
  },
};

package.json

{
  "name": "test"
}

wrangler.toml

name = "test"
pages_build_output_dir = "."
[vars]
ASSETS = "test"

Then running

pnpx wrangler pages deploy 
โœ” Select an account โ€บ Pete Bacon Darwin Account
๐ŸŒ  Uploading... (2/2)

โœจ Success! Uploaded 1 files (1 already uploaded) (1.22 sec)

โœจ Compiled Worker successfully
โœจ Uploading Worker bundle
๐ŸŒŽ Deploying...

โœ˜ [ERROR] Deployment failed!

  Failed to publish your Function. Got error: Unknown internal error occurred.
petebacondarwin commented 4 months ago

Really we should be getting a better error back from the API when this fails, but we could perhaps put an additional validation check into Wrangler.