fix wrangler dev logs being logged on the incorrect level in some cases

[dario-piotrowicz]

Fixed https://jira.cfdata.org/browse/DEVX-2087
Fixes #8063

---

• Tests
  • Tests included
  • Tests not necessary because:
• Public documentation
  • Cloudflare docs PR(s):
  • Documentation not necessary because: bugfix
• Wrangler V3 Backport
  • Wrangler PR: V3 Backport [#10004]: fix wrangler dev logs being logged on the incorrect level in some cases #10015
  • Not necessary because:

[changeset-bot]

🦋 Changeset detected

Latest commit: b3ccc35

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

This PR includes changesets to release 5 packages

Name	Type
wrangler	Patch
miniflare	Minor
@cloudflare/vite-plugin	Patch
@cloudflare/vitest-pool-workers	Patch
@cloudflare/pages-shared	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

[pkg-pr-new]

create-cloudflare

npm i https://pkg.pr.new/create-cloudflare@10004

@cloudflare/kv-asset-handler

npm i https://pkg.pr.new/@cloudflare/kv-asset-handler@10004

miniflare

npm i https://pkg.pr.new/miniflare@10004

@cloudflare/pages-shared

npm i https://pkg.pr.new/@cloudflare/pages-shared@10004

@cloudflare/unenv-preset

npm i https://pkg.pr.new/@cloudflare/unenv-preset@10004

@cloudflare/vite-plugin

npm i https://pkg.pr.new/@cloudflare/vite-plugin@10004

@cloudflare/vitest-pool-workers

npm i https://pkg.pr.new/@cloudflare/vitest-pool-workers@10004

@cloudflare/workers-editor-shared

npm i https://pkg.pr.new/@cloudflare/workers-editor-shared@10004

wrangler

npm i https://pkg.pr.new/wrangler@10004

commit: b3ccc35

[dario-piotrowicz]

@danlapid I've added you as a reviewer just in case you were interested since you added the structured logging feature to workerd 🙌 🫶

[danlapid]

@danlapid I've added you as a reviewer just in case you were interested since you added the structured logging feature to workerd 🙌 🫶

🥳 🥳 🥳
Did everything work well for you with it? Any issues?

[dario-piotrowicz]

@danlapid I've added you as a reviewer just in case you were interested since you added the structured logging feature to workerd 🙌 🫶

🥳 🥳 🥳 Did everything work well for you with it? Any issues?

Hey @danlapid 🙂

All very good generally thanks 🫶

One thing that I thought could be a bit problematic but I am getting worried that it could be quite problematic is this regex: https://github.com/cloudflare/workers-sdk/blob/fdf3ecbf8b07c5193a0d5215e4c193059719182a/packages/wrangler/src/dev/miniflare.ts#L1192C3-L1192C74

I can be quite brittle.... I will try to make it more robust (I need to add some more testing around this 🤔)

But however much I can improve it, I feel like it can always be a bit brittle to some extent 🤔 (since user logs can literally have anything in them...)
I was considering.... do you think we could also add a workerd option to have the message in the structured logs be in base64 or something to that extent? (so that we could extract the message and convert it back to text format on our end) that would make this sort of operation very robust I think 🤔

[danlapid]

@danlapid I've added you as a reviewer just in case you were interested since you added the structured logging feature to workerd 🙌 🫶

🥳 🥳 🥳 Did everything work well for you with it? Any issues?

Hey @danlapid 🙂

All very good generally thanks 🫶

One thing that I thought could be a bit problematic but I am getting worried that it could be quite problematic is this regex: https://github.com/cloudflare/workers-sdk/blob/fdf3ecbf8b07c5193a0d5215e4c193059719182a/packages/wrangler/src/dev/miniflare.ts#L1192C3-L1192C74

I can be quite brittle.... I will try to make it more robust (I need to add some more testing around this 🤔)

But however much I can improve it, I feel like it can always be a bit brittle to some extent 🤔 (since user logs can literally have anything in them...) I was considering.... do you think we could also add a workerd option to have the message in the structured logs be in base64 or something to that extent? (so that we could extract the message and convert it back to text format on our end) that would make this sort of operation very robust I think 🤔

Why not just use a json parser?

[dario-piotrowicz]

Why not just use a json parser?

mh... yeah that's a good point! Thanks I'll try that 🙂👍

[dario-piotrowicz]

Why not just split by newlines though? Are you worried that the logs can have newlines? I am pretty sure these should be correctly escaped by our json encoder.

Nice! I actually didn't know that they were consistently divided by newlines 😃

Thanks for the suggestion, I've applied that and it seems to work very nicely 🫶

[emily-shen]

this is all auto-generated right? but do you why the DO stuff has been added in? is this just because we haven't updated this file recently since the facets stuff was added to the runtime? i wonder if this is something we should automate

[dario-piotrowicz]

yeah this was automatically generated via

workers-sdk/packages/miniflare/package.json

Line 34 in 031c7f6

"capnp:workerd": "node scripts/build-capnp.mjs",

honestly I haven't really looked much at the changes in the file 😕 (as it just works and doesn't seem to break anything)

I guess it can be periodically automated, but I think that there's no much benefit in it since the file only needs to update when miniflare needs to pass new/different capnp configs, and that updating it only when necessary makes sense to me 🤔
(In other words, I think that the file getting updated when Miniflare doesn't use the new options is not going to have any effect anyways)

I'd be keen to hear what @penalosa thinks of this 🙂

[dario-piotrowicz]

PR converted to draft as I think it now needs to wait for cloudflare/workerd#4620 to be merged (released and bumped in wrangler).

Also a test for top level errors (like the one I mentioned above) that would get logged onto stderr, should be added.

[dario-piotrowicz]

I've added the skip-v3-pr label just to make the CI happy, I do already have the v3 PR open, once this PR is merged I will fix the v3 PR