Follow up: add log prefix - #2293
Conversation
|
👋 cedric-cordenier, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
📊 API Diff Results
|
There was a problem hiding this comment.
Pull request overview
This PR adjusts host-side handling of WASM guest log messages so that guest-requested panic/fatal log levels are still downgraded to error (to avoid crashing/exiting the host), but are now distinguishable via a prefix in the log message.
Changes:
- Add explicit message prefixes when guest logs at
panicorfatallevel (while still logging aterror). - Relax the existing test assertion to no longer require an exact log message match.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/workflows/wasm/host/module.go | Adds distinct prefixes for downgraded panic/fatal guest log messages. |
| pkg/workflows/wasm/host/logging_test.go | Updates the test assertion for panic/fatal logging behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| case "panic": | ||
| // The guest should never be able to panic/exit the host | ||
| logger.Errorw(sanitizedMsg, args...) | ||
| logger.Errorw("[panic] "+sanitizedMsg, args...) | ||
| case "fatal": | ||
| // The guest should never be able to panic/exit the host | ||
| logger.Errorw("[fatal] "+sanitizedMsg, args...) |
| entries := logs.All() | ||
| require.Len(t, entries, 1) | ||
| require.Equal(t, zapcore.ErrorLevel, entries[0].Level) | ||
| require.Equal(t, "boom", entries[0].Message) | ||
| require.Contains(t, entries[0].Message, "boom") | ||
| }) |
1c9ff32 to
3af125b
Compare
Requires
Supports