worker fixes (#4059)

Describe what your pull request does. If you can, add GIFs or images
showing the before and after of your change.

### Change type

- [x] `bugfix`
- [ ] `improvement`
- [ ] `feature`
- [ ] `api`
- [ ] `other`

### Test plan

1. Create a shape...
2.

- [ ] Unit tests
- [ ] End to end tests

### Release notes

- Fixed a bug with...

---------

Co-authored-by: alex <alex@dytry.ch>

apps/bemo-worker/src/BemoDO.ts

@@ -8,7 +8,7 @@ export class BemoDO extends DurableObject<Environment> {
 		const sentry = createSentry(this.ctx, this.env, request)
 		console.error(e)
 		// eslint-disable-next-line deprecation/deprecation
-		sentry.captureException(e)
+		sentry?.captureException(e)
 	}
 
 	private readonly router = Router()

apps/bemo-worker/src/worker.ts

@@ -27,7 +27,7 @@ export default class Worker extends WorkerEntrypoint<Environment> {
 			const sentry = createSentry(this.ctx, this.env, request)
 			console.error(error)
 			// eslint-disable-next-line deprecation/deprecation
-			sentry.captureException(error)
+			sentry?.captureException(error)
 			return new Response('Something went wrong', {
 				status: 500,
 				statusText: 'Internal Server Error',

apps/dotcom-asset-upload/package.json

@@ -9,7 +9,7 @@
 	},
 	"main": "src/index.ts",
 	"scripts": {
-		"dev": "cross-env NODE_ENV=development wrangler dev --log-level info --persist-to tmp-assets",
+		"dev": "yarn run -T tsx ../../scripts/workers/dev.ts --persist-to tmp-assets",
 		"test-ci": "lazy inherit --passWithNoTests",
 		"test": "yarn run -T jest --passWithNoTests",
 		"test-coverage": "lazy inherit --passWithNoTests",

apps/dotcom-asset-upload/wrangler.toml

@@ -1,4 +1,4 @@
-main = "src/index.ts"
+main = "src/worker.ts"
 compatibility_date = "2022-09-22"
 
 [dev]
@@ -34,7 +34,7 @@ preview_bucket_name = 'uploads-preview'
 binding = 'UPLOADS'
 bucket_name = 'uploads-preview'
 
-[[env.stating.r2_buckets]]
+[[env.staging.r2_buckets]]
 binding = 'UPLOADS'
 bucket_name = 'uploads-preview'
 

apps/dotcom-worker/src/TLDrawDurableObject.ts

@@ -222,7 +222,7 @@ export class TLDrawDurableObject {
 		} catch (err) {
 			console.error(err)
 			// eslint-disable-next-line deprecation/deprecation
-			sentry.captureException(err)
+			sentry?.captureException(err)
 			return new Response('Something went wrong', {
 				status: 500,
 				statusText: 'Internal Server Error',

apps/dotcom-worker/src/worker.ts

@@ -62,7 +62,7 @@ const Worker = {
 			.catch((err) => {
 				console.error(err)
 				// eslint-disable-next-line deprecation/deprecation
-				sentry.captureException(err)
+				sentry?.captureException(err)
 
 				return new Response('Something went wrong', {
 					status: 500,

packages/worker-shared/src/sentry.ts

@@ -9,11 +9,17 @@ interface Context {
 
 interface SentryEnvironment {
 	readonly SENTRY_DSN: string | undefined
+	readonly TLDRAW_ENV?: string | undefined
 	readonly WORKER_NAME: string | undefined
 	readonly CF_VERSION_METADATA: WorkerVersionMetadata
 }
 
 export function createSentry(ctx: Context, env: SentryEnvironment, request?: Request) {
+	// TLDRAW_ENV is undefined in dev
+	if (!env.SENTRY_DSN && !env.TLDRAW_ENV) {
+		return null
+	}
+
 	const { SENTRY_DSN, WORKER_NAME, CF_VERSION_METADATA } = requiredEnv(env, {
 		SENTRY_DSN: true,
 		WORKER_NAME: true,

scripts/package.json

@@ -55,9 +55,11 @@
 	"dependencies": {
 		"@octokit/rest": "^20.0.2",
 		"@types/minimist": "^1.2.5",
+		"@types/proper-lockfile": "^4.1.4",
 		"@types/tmp": "^0.2.6",
 		"ignore": "^5.2.4",
 		"minimist": "^1.2.8",
+		"proper-lockfile": "^4.1.2",
 		"tar": "^7.0.1",
 		"tmp": "^0.2.3",
 		"toml": "^3.0.0"

scripts/workers/dev.ts

@@ -1,11 +1,20 @@
 import { ChildProcessWithoutNullStreams, spawn } from 'child_process'
 import kleur from 'kleur'
+import { lock } from 'proper-lockfile'
 import stripAnsi from 'strip-ansi'
 
-// at the time of writing, workerd will regularly crash with a segfault
+const lockfileName = __dirname
-// but the error is not caught by the process, so it will just hang
+
-// this script wraps the process, tailing the logs and restarting the process
+/**
-// if we encounter the string 'Segmentation fault'
+ * a long time ago, workerd would regularly crash with a segfault but the error is not caught by the
+ * process, so it will just hang. this script wraps the process, tailing the logs and restarting the
+ * process if we encounter the string 'Segmentation fault'. we think this error is probably fixed
+ * now.
+ *
+ * there's a separate issue where spawning multiple workerd instances at once would cause them to
+ * fight and crash. we use a lockfile to start our wrokerd instances one at a time, waiting for the
+ * previous one to be ready before we start the next.
+ */
 class MiniflareMonitor {
 	private process: ChildProcessWithoutNullStreams | null = null
 
@@ -14,9 +23,10 @@ class MiniflareMonitor {
 		private args: string[] = []
 	) {}
 
-	public start(): void {
+	public async start(): Promise<void> {
-		this.stop() // Ensure any existing process is stopped
+		await this.stop() // Ensure any existing process is stopped
-		console.log(`Starting wrangler...`)
+		await this.lock()
+		await console.log(`Starting wrangler...`)
 		this.process = spawn(this.command, this.args, {
 			env: {
 				NODE_ENV: 'development',
@@ -35,6 +45,11 @@ class MiniflareMonitor {
 
 	private handleOutput(output: string, err = false): void {
 		if (!output) return
+
+		if (output.includes('Ready on') && this.isLocked()) {
+			this.release()
+		}
+
 		if (output.includes('Segmentation fault')) {
 			console.error('Segmentation fault detected. Restarting Miniflare...')
 			this.restart()
@@ -43,18 +58,45 @@ class MiniflareMonitor {
 		}
 	}
 
-	private restart(): void {
+	private async restart(): Promise<void> {
 		console.log('Restarting wrangler...')
-		this.stop()
+		await this.stop()
 		setTimeout(() => this.start(), 3000) // Restart after a short delay
 	}
 
-	private stop(): void {
+	private async stop(): Promise<void> {
+		if (this.isLocked()) await this.release()
 		if (this.process) {
 			this.process.kill()
 			this.process = null
 		}
 	}
+
+	private _lockPromise?: Promise<() => Promise<void>>
+	private isLocked() {
+		return !!this._lockPromise
+	}
+	private async lock() {
+		if (this.isLocked()) throw new Error('Already locked')
+		console.log('Locking...')
+		this._lockPromise = lock(lockfileName, {
+			retries: {
+				minTimeout: 500,
+				retries: 10,
+			},
+		})
+		await this._lockPromise
+		console.log('Locked')
+	}
+	private async release() {
+		if (!this.isLocked()) throw new Error('Not locked')
+		console.log('Releasing...')
+		const lockPromise = this._lockPromise!
+		this._lockPromise = undefined
+		const release = await lockPromise
+		await release()
+		console.log('Released')
+	}
 }
 
 class SizeReporter {
@@ -119,6 +161,7 @@ new MiniflareMonitor('wrangler', [
 	'info',
 	'--var',
 	'IS_LOCAL:true',
+	...process.argv.slice(2),
 ]).start()
 
 new SizeReporter().start()

yarn.lock

@@ -6196,6 +6196,7 @@ __metadata:
     "@types/is-ci": "npm:^3.0.0"
     "@types/minimist": "npm:^1.2.5"
     "@types/node": "npm:~20.11"
+    "@types/proper-lockfile": "npm:^4.1.4"
     "@types/tmp": "npm:^0.2.6"
     "@typescript-eslint/utils": "npm:^5.59.0"
     ast-types: "npm:^0.14.2"
@@ -6210,6 +6211,7 @@ __metadata:
     lazyrepo: "npm:0.0.0-alpha.27"
     minimist: "npm:^1.2.8"
     prettier: "npm:^3.0.3"
+    proper-lockfile: "npm:^4.1.2"
     recast: "npm:^0.22.0"
     rimraf: "npm:^4.4.0"
     semver: "npm:^7.3.8"
@@ -6936,6 +6938,15 @@ __metadata:
   languageName: node
   linkType: hard
 
+"@types/proper-lockfile@npm:^4.1.4":
+  version: 4.1.4
+  resolution: "@types/proper-lockfile@npm:4.1.4"
+  dependencies:
+    "@types/retry": "npm:*"
+  checksum: b0d1b8e84a563b2c5f869f7ff7542b1d83dec03d1c9d980847cbb189865f44b4a854673cdde59767e41bcb8c31932e613ac43822d358a6f8eede6b79ccfceb1d
+  languageName: node
+  linkType: hard
+
 "@types/qrcode@npm:^1.5.0":
   version: 1.5.5
   resolution: "@types/qrcode@npm:1.5.5"
@@ -7004,6 +7015,13 @@ __metadata:
   languageName: node
   linkType: hard
 
+"@types/retry@npm:*":
+  version: 0.12.5
+  resolution: "@types/retry@npm:0.12.5"
+  checksum: 3fb6bf91835ca0eb2987567d6977585235a7567f8aeb38b34a8bb7bbee57ac050ed6f04b9998cda29701b8c893f5dfe315869bc54ac17e536c9235637fe351a2
+  languageName: node
+  linkType: hard
+
 "@types/scheduler@npm:*":
   version: 0.16.8
   resolution: "@types/scheduler@npm:0.16.8"
@@ -19011,6 +19029,17 @@ __metadata:
   languageName: node
   linkType: hard
 
+"proper-lockfile@npm:^4.1.2":
+  version: 4.1.2
+  resolution: "proper-lockfile@npm:4.1.2"
+  dependencies:
+    graceful-fs: "npm:^4.2.4"
+    retry: "npm:^0.12.0"
+    signal-exit: "npm:^3.0.2"
+  checksum: 000a4875f543f591872b36ca94531af8a6463ddb0174f41c0b004d19e231d7445268b422ff1ea595e43d238655c702250cd3d27f408e7b9d97b56f1533ba26bf
+  languageName: node
+  linkType: hard
+
 "property-information@npm:^6.0.0":
   version: 6.4.0
   resolution: "property-information@npm:6.4.0"