feat: change access-api wrangler.toml to be no_bundle=false. use wrangler bundling #739

gobengo · 2023-04-11T01:28:59Z

Motivation:

access-api stacktraces in sentry are not working #623
verify our assumption that we have to use no_bundle=true along with d1.

Experiment

I wanted to try some new ways of building and whether they would work, but without risking staging, so I used the 'dev' environment from wrangler.toml to try some deploys from local.
I made access-api use of wrangler more like toucan-js@3 wrangler-basic
I did a sample deploy from my laptop to 'dev' workers environment, with the sentry/toucan release called bengo-dev-0 sentry errors here
trigger error via GET /.debug/error
- expectation: this to lead to an in-route error and show up in sentry with helpful stack traces - because this is what example implies should work
trigger error via route that uses d1 binding (e.g. ucanto routes triggered via w3up+ucanto observable pointing to dev env)
- expectation: d1 error - because supposedly no_bundle=false (or omitting no_bundle) is incompatible with wrangler d1 bindings

Findings:

omg it works!
error triggered by GET /.debug/error (no d1)
error triggered by using d1 via access protocol via observable
- The error did not happen in the access/authorize handler. That worked fine, and I got an email from dev env. The error happened when I clicked the email (invoking access/confirm), when the access/confirm invoacation handler tried to write to d1
- It got a D1_ERROR, but I could see from the sentry report
  
  Error: ERROR 9009: SQL prepare error: no such table: delegations_v3
- which makes sense, because dev doesn't have latest migrations applied that created that table. So I provisioned a new d1 database for this dev env, and ran npx wrangler --env=dev d1 migrations apply __D1_BETA__. This would error every couple migrations, but if I re-ran it it would make progress and error again (Internal error [code: 7501]), but eventually it would succeed at all migrations.
- then I re-triggered things via the observable, get email, click email. I saw 'email validated'! 🎉
- I continued using the observable to invoke access/claim (reads from D1), and got no error all the way through invoking from second device

…uilt-in bundling

gobengo · 2023-04-13T16:14:55Z

packages/access-api/wrangler.toml


 [vars]
 ENV = "dev"
 DEBUG = "true"
 DID = "did:web:local.web3.storage"
+PRIVATE_KEY="MgCYWjE6vp0cn3amPan2xPO+f6EZ3I+KwuN1w2vx57vpJ9O0Bn4ci4jn8itwc121ujm7lDHkCW24LuKfZwIdmsifVysY="


This is the same value from repo /.env.tpl , so it is already public

…in bundling)

…oy requries it)

gobengo · 2023-04-13T16:55:18Z

create env.ACCOUNT_VERSION string at build time, and have access-api pass it to toucan as opts.release - previously this would get injected via esbuild at custom build time

gobengo · 2023-04-14T01:05:19Z

How I tested this:

"manual deploy" this source branch to dev env
created in sentry release @web3-storage__access-api@6.0.0-dev+0be6257
GET https://w3access-dev.protocol-labs.workers.dev/.debug/error to trigger an error
error in sentry

olizilla

Is good. Comments inline but nothing blocking, so approving this on the assumption you will tweak or explain as required.

olizilla · 2023-04-14T09:19:28Z

.github/workflows/reusable-deploy-api.yml

+        working-directory: packages/access-api
+        run: |
+          # write ./dist/index.{js,js.map}
+          pnpm exec wrangler --env="$ENV" publish --dry-run --outdir=dist


my assumption here is that the step before should already generate these as part of the wrangler action. I'd feel more comfortable here if the artefacts are build once and then sent to the places they need to go, rather than relying on the build definitely always being reproducible (it should be, but, if why tempt fate in our error reporting)

what does SENTRY_UPLOAD do in that previous step? is it unused, or does wrangler-action actually upload a thing to sentry (that would be surprising)

what does SENTRY_UPLOAD do in that previous step? is it unused, or does wrangler-action actually upload a thing to sentry (that would be surprising)

Nothing atm. (because now wrangler.toml doesn't define a custom build command that might read it).

my assumption here is that the step before should already generate these as part of the wrangler action.

Good suggestion. I had thought so too, and tried that, but iirc it didn't write the files.

Let me see if it's just because I need to pass --outdir=dist on the previous non-dry-run publish.

Added a commit to only invoke wrangler publish once (via wrangler-action with custom command to pass --outdir dist) 3af1c35

packages/access-api/src/utils/context.js

olizilla · 2023-04-14T09:26:22Z

packages/access-api/scripts/release.js

+async function main(argv = process.argv) {
+  const cli = sade('access-api-release')
+  cli
+    .command('name', '', { default: true })
+    .option('--env', 'Environment', process.env.ENV)
+    .action((opts) => {
+      const releaseName = getReleaseName(opts.env)
+      console.log(releaseName)
+    })
+  cli
+    .command('esm', 'print release info as ES Module string')
+    .option('--env', 'Environment', process.env.ENV)
+    .action((opts) => {
+      const lines = [
+        `/** @type {string|undefined} */`,
+        `export const gitRevShort = '${git.short(__dirname())}'`,
+        `/** @type {string|undefined} */`,
+        `export const name = '${getReleaseName(opts.env)}'`,
+      ]
+      console.log(lines.join('\n'))
+    })
+  cli.parse(argv)
+}


this is neat, but note that our other projects do this by injecting these in as variables into the bundle at build time.

They do that using no_bundle=true and injection via custom esbuild, so this is a way of doing it with no_bundle=false.
Yes it's different, but it seems worth it in order to get the stack traces.

If there's another way that works that seems better, lmk and I can try to implement.

olizilla · 2023-04-14T09:28:41Z

packages/access-api/wrangler.toml

+rules = [
+  { type = "Text", globs = ["package.json"], fallthrough = true }
+]


I believe this was required for cloudflare workers to be able to import the package.json, which
I think it's a bundler-independent way of providing enough information about whether importing from that json file should result in a string Text or a json-parsed object.
https://developers.cloudflare.com/workers/wrangler/configuration/#bundling

I don't think I need it anymore, since now release.node.js imports it in node at build time, and writes a rendered value for cloudflare to import via ESM (so it doesn't need to import the package.json to build the rendered version).

I'll remove it

olizilla · 2023-04-14T09:29:38Z

packages/access-api/wrangler.toml

@@ -28,43 +25,52 @@ database_id = "7c676e0c-b9e7-4711-97c8-7b1c8eb229ae"
 [[r2_buckets]]
 binding = "DELEGATIONS_BUCKET"
 bucket_name = "w3up-delegations-dev-0"
+preview_bucket_name = "w3up-delegations-dev-0"


some changes in this file seem unrelated to bundling / getting stack traces to work. not a blocker, but can you confirm why these changes are needed in this PR?

This one was suggested by wrangler cli when I was deploying to dev env. The dev env has been usused for a bit, so I think we just hadn't exercised that path, so this makes sense as an addition that makes deploying to dev better (seemingly unrelated to d1/sentry/bundling, but the relationship is that the dev env is useful for testing deployments/new-code without disrupting staging, so it was useful here, and is probably useful to commit so others dont run into it when they try to use dev env + wrangler)

gobengo · 2023-04-14T18:15:15Z

error from staging after this deployed there

🤖 I have created a release *beep* *boop* --- ## [6.1.0](access-api-v6.0.0...access-api-v6.1.0) (2023-04-14) ### Features * change access-api wrangler.toml to be no_bundle=false. use wrangler bundling ([#739](#739)) ([d659516](d659516)) ### Bug Fixes * access-api cli.js build does sourcemap=external instead of true ([#731](#731)) ([d1a35b7](d1a35b7)) * add /.debug/error route ([#732](#732)) ([2ca5de8](2ca5de8)) * change access-api src/index.js 'export default' to be an immediately-defined object in hopes stack traces will work better ([#733](#733)) ([a509313](a509313)), closes [#623](#623) * toucan-js in access-api does not use RewriteFrame integration ([#729](#729)) ([eeb90e6](eeb90e6)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

…use wrangler bundling (#739)" This reverts commit d659516. I believe this commit resulted in the Postmark API key not being set in staging and production - reverting to test.

change access-api wrangler.toml to be no_bundle=false. use wrangler b…

1ffb681

…uilt-in bundling

gobengo changed the title ~~change access-api wrangler.toml to be no_bundle=false. use wrangler bundling~~ feat: change access-api wrangler.toml to be no_bundle=false. use wrangler bundling Apr 11, 2023

prettier

7cf6278

gobengo marked this pull request as ready for review April 11, 2023 21:06

gobengo commented Apr 13, 2023

View reviewed changes

gobengo requested review from Gozala and olizilla April 13, 2023 16:15

rm build step from access-api envs dev,staging,production (use built-…

4ec1075

…in bundling)

gobengo had a problem deploying to staging April 13, 2023 16:34 — with GitHub Actions Error

gobengo had a problem deploying to dev April 13, 2023 16:35 — with GitHub Actions Failure

create new dev d1 db database_id for name access-dev (cloudflare depl…

53ff41f

…oy requries it)

gobengo temporarily deployed to dev April 13, 2023 16:42 — with GitHub Actions Inactive

gobengo added 4 commits April 13, 2023 10:38

make sentry-release-version code a reusable function/script

92a89d3

wip release scripts

9570179

wip

6927148

wip

27eff7b

gobengo temporarily deployed to dev April 13, 2023 21:58 — with GitHub Actions Inactive

gobengo added 2 commits April 13, 2023 15:07

add release name prep to reusable-deploy-api

8465228

prettier

df8d799

gobengo had a problem deploying to dev April 13, 2023 22:41 — with GitHub Actions Failure

gobengo added 2 commits April 13, 2023 15:47

fix how reusable-deploy-api gets release name

87bd214

reusable-deploy-api only installs packages on dep tree of access-api

9cb44c3

gobengo had a problem deploying to dev April 13, 2023 22:48 — with GitHub Actions Failure

reusable-deploy-api publishes sentry release and source maps

ba143bd

gobengo had a problem deploying to dev April 13, 2023 22:57 — with GitHub Actions Failure

gobengo had a problem deploying to dev April 13, 2023 23:40 — with GitHub Actions Failure

wip

f3446a0

gobengo had a problem deploying to dev April 13, 2023 23:48 — with GitHub Actions Failure

wip

50ded92

gobengo had a problem deploying to dev April 13, 2023 23:55 — with GitHub Actions Failure

wip

25aecb0

gobengo temporarily deployed to dev April 14, 2023 00:02 — with GitHub Actions Inactive

wip

6f16f09

gobengo had a problem deploying to staging April 14, 2023 00:11 — with GitHub Actions Error

gobengo had a problem deploying to dev April 14, 2023 00:12 — with GitHub Actions Failure

wip

fb13a84

gobengo temporarily deployed to dev April 14, 2023 00:16 — with GitHub Actions Inactive

remove extra quotes

0be6257

gobengo temporarily deployed to dev April 14, 2023 00:50 — with GitHub Actions Inactive

gobengo added 2 commits April 13, 2023 17:58

clean up reusable-deploy-api.yml

19b7cd3

typo

3cecdf6

gobengo requested a review from vasco-santos April 14, 2023 01:05

gobengo mentioned this pull request Apr 14, 2023

Source maps still don't seem to be working robertcepa/toucan-js#54

Closed

olizilla approved these changes Apr 14, 2023

View reviewed changes

gobengo added 3 commits April 14, 2023 10:38

remove console

7aabcc0

access-api wranger.toml doesn't specify rules unnecessarily

1d7f23e

try only running wrangler publish once

3af1c35

gobengo temporarily deployed to dev April 14, 2023 17:48 — with GitHub Actions Inactive

dont pass unnecessary env vars to wrangler publish

d58f3b6

gobengo deployed to dev April 14, 2023 17:56 — with GitHub Actions Active

gobengo merged commit d659516 into main Apr 14, 2023

gobengo deleted the 623-no-sentry-ci-push-4 branch April 14, 2023 18:06

it-dag-house mentioned this pull request Apr 14, 2023

chore(main): release access-api 6.1.0 #757

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: change access-api wrangler.toml to be no_bundle=false. use wrangler bundling #739

feat: change access-api wrangler.toml to be no_bundle=false. use wrangler bundling #739

gobengo commented Apr 11, 2023 •

edited

Loading

gobengo Apr 13, 2023

gobengo commented Apr 13, 2023 •

edited

Loading

gobengo commented Apr 14, 2023

olizilla left a comment

olizilla Apr 14, 2023

gobengo Apr 14, 2023

gobengo Apr 14, 2023

gobengo Apr 14, 2023

olizilla Apr 14, 2023

gobengo Apr 14, 2023

olizilla Apr 14, 2023

gobengo Apr 14, 2023 •

edited

Loading

gobengo Apr 14, 2023

olizilla Apr 14, 2023

gobengo Apr 14, 2023

gobengo commented Apr 14, 2023

feat: change access-api wrangler.toml to be no_bundle=false. use wrangler bundling #739

feat: change access-api wrangler.toml to be no_bundle=false. use wrangler bundling #739

Conversation

gobengo commented Apr 11, 2023 • edited Loading

Choose a reason for hiding this comment

gobengo commented Apr 13, 2023 • edited Loading

gobengo commented Apr 14, 2023

olizilla left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gobengo Apr 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gobengo commented Apr 14, 2023

gobengo commented Apr 11, 2023 •

edited

Loading

gobengo commented Apr 13, 2023 •

edited

Loading

gobengo Apr 14, 2023 •

edited

Loading