Conversation
mihaitodor
commented
May 12, 2025
- Only update Benthos and Connect automatically
- Make sure we parse the output correctly when the mapping returns a simple byte array
- Make sure we pull in the latest wasm_exec.js
- Update the Readme
✅ Deploy Preview for docs-ui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis set of changes updates the build, dependency management, and code quality workflows for the Bloblang Playground's WebAssembly (Wasm) module and related frontend integration. The GitHub Actions workflows are modified to target specific Go dependencies for updates and to introduce Go linting. The Wasm module's Go code is refactored for improved error handling and output formatting, and its module name is changed. The frontend JavaScript handling of Wasm output is adjusted, and the build process now copies the Go runtime's Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend (JS)
participant Wasm Module (Go)
participant GitHub Actions
User->>Frontend (JS): Initiates Bloblang Playground action
Frontend (JS)->>Wasm Module (Go): Calls blobl() with args
Wasm Module (Go)->>Wasm Module (Go): Processes args, applies mapping, handles metadata
Wasm Module (Go)->>Frontend (JS): Returns JSON result (msg, meta)
Frontend (JS)->>Frontend (JS): Extracts msg and meta from result
Frontend (JS)->>User: Displays output
Note over GitHub Actions: Workflow for build/validation
GitHub Actions->>GitHub Actions: Checkout, setup Go, update specific dependencies, tidy modules
GitHub Actions->>GitHub Actions: Run Go linter
GitHub Actions->>GitHub Actions: Setup Node, bundle UI
Possibly related PRs
Suggested reviewers
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
| go get github.com/redpanda-data/benthos/v4 | ||
| go get github.com/redpanda-data/connect/v4 |
There was a problem hiding this comment.
I think this should be fine... All the other dependencies are implicit so they should be updated when we update these.
| "encoding/json" | ||
| "fmt" | ||
| "syscall/js" | ||
| "encoding/json" |
There was a problem hiding this comment.
Somehow, this file had the tabs converted to spaces in a previous PR. Not sure why...
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
gulpfile.js (1)
90-107:⚠️ Potential issue
done()is invoked twice – Gulp will error outBoth
execcalls use the same callback:
- Copy
wasm_exec.js→done()- Build
.wasm→done()When the first finishes, the task signals completion, but the second will later call
done()again, causing “task callback called too many times”.- exec('cp "$(go env GOROOT)/lib/wasm/wasm_exec.js" ./src/js/vendor', { cwd: __dirname }, (err, stdout, stderr) => { - if (err) { …; done(err); return } - done() - }) - - exec(`go build -o "${wasmOutputPath}" .`, { cwd: wasmDir, env: envVars }, (err, stdout, stderr) => { - if (err) { …; done(err); return } - … - done() - }) + exec( + 'cp "$(go env GOROOT)/lib/wasm/wasm_exec.js" ./src/js/vendor', + { cwd: __dirname }, + (copyErr, _1, copyStderr) => { + if (copyErr) { + log.error('Error copying wasm_exec.js:', copyStderr); + return done(copyErr); + } + exec( + `go build -o "${wasmOutputPath}" .`, + { cwd: wasmDir, env: envVars }, + (buildErr, stdout, stderr) => { + if (buildErr) { + log.error('Error building WASM:', stderr); + return done(buildErr); + } + log.info('WebAssembly built successfully:', stdout); + done(); // called exactly once + } + ); + } + );While here, consider replacing the shell-specific
cp "$(go env …)"with a Node copy (fs.copyFileSync) pluschild_process.execSync('go env GOROOT'); this is portable across Windows and *nix.
🧹 Nitpick comments (1)
src/partials/bloblang-playground.hbs (1)
307-309: Verify WebAssembly integration.The
bloblfunction is called here, but there's no clear error handling for when the WebAssembly module isn't properly loaded. Consider adding a check before calling this function.- const result = blobl(getMapping(), getInput(), getInputMetadata()); + if (typeof blobl !== 'function') { + throw new Error("WebAssembly module not properly loaded. Please refresh the page."); + } + const result = blobl(getMapping(), getInput(), getInputMetadata());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.github/workflows/update-go-mod.yml(1 hunks).github/workflows/validate-build.yml(1 hunks)blobl-editor/README.md(1 hunks)blobl-editor/wasm/go.mod(1 hunks)blobl-editor/wasm/main.go(1 hunks)gulpfile.js(1 hunks)src/js/vendor/wasm_exec.js(3 hunks)src/partials/bloblang-playground.hbs(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/validate-build.yml
13-13: the runner of "actions/setup-go@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 golangci-lint (1.64.8)
blobl-editor/wasm/main.go
6-6: could not import syscall/js (-: build constraints exclude all Go files in /tmp/go/.go-mod-cache/golang.org/[email protected]/src/syscall/js)
(typecheck)
🪛 ESLint
src/js/vendor/wasm_exec.js
[error] 17-17: Unexpected tab character.
(no-tabs)
[error] 17-17: Expected indentation of 6 spaces but found 3 tabs.
(indent)
[error] 76-76: Unexpected tab character.
(no-tabs)
[error] 76-76: Expected indentation of 2 spaces but found 1 tab.
(indent)
[error] 76-76: 'globalThis' is not defined.
(no-undef)
[error] 77-77: Unexpected tab character.
(no-tabs)
[error] 77-77: Expected indentation of 4 spaces but found 2 tabs.
(indent)
[error] 77-77: 'globalThis' is not defined.
(no-undef)
[error] 78-78: Unexpected tab character.
(no-tabs)
[error] 78-78: Expected indentation of 6 spaces but found 3 tabs.
(indent)
[error] 78-78: Missing space before function parentheses.
(space-before-function-paren)
[error] 79-79: Unexpected tab character.
(no-tabs)
[error] 79-79: Expected indentation of 8 spaces but found 4 tabs.
(indent)
[error] 79-79: Strings must use singlequote.
(quotes)
[error] 79-79: Extra semicolon.
(semi)
[error] 80-80: Unexpected tab character.
(no-tabs)
[error] 80-80: Expected indentation of 6 spaces but found 3 tabs.
(indent)
[error] 80-81: Missing trailing comma.
(comma-dangle)
[error] 81-81: Unexpected tab character.
(no-tabs)
[error] 81-81: Expected indentation of 4 spaces but found 2 tabs.
(indent)
[error] 82-82: Unexpected tab character.
(no-tabs)
[error] 82-82: Expected indentation of 2 spaces but found 1 tab.
(indent)
[error] 219-219: Unexpected tab character.
(no-tabs)
[error] 219-219: Expected indentation of 6 spaces but found 3 tabs.
(indent)
[error] 220-220: Unexpected tab character.
(no-tabs)
[error] 220-220: Expected indentation of 8 spaces but found 4 tabs.
(indent)
[error] 220-220: Extra semicolon.
(semi)
[error] 221-221: Unexpected tab character.
(no-tabs)
[error] 221-221: Expected indentation of 8 spaces but found 4 tabs.
(indent)
[error] 221-221: Extra semicolon.
(semi)
[error] 222-222: Unexpected tab character.
(no-tabs)
[error] 222-222: Expected indentation of 6 spaces but found 3 tabs.
(indent)
[error] 224-224: Unexpected tab character.
(no-tabs)
[error] 224-224: Expected indentation of 6 spaces but found 3 tabs.
(indent)
[error] 224-224: 'performance' is not defined.
(no-undef)
[error] 224-224: Extra semicolon.
(semi)
[error] 225-225: Unexpected tab character.
(no-tabs)
[error] 225-225: Expected indentation of 6 spaces but found 3 tabs.
(indent)
[error] 226-226: Unexpected tab character.
(no-tabs)
[error] 226-226: Expected indentation of 8 spaces but found 4 tabs.
(indent)
[error] 227-227: Unexpected tab character.
(no-tabs)
[error] 227-227: Expected indentation of 10 spaces but found 5 tabs.
(indent)
[error] 228-228: Unexpected tab character.
(no-tabs)
[error] 228-228: Expected indentation of 10 spaces but found 5 tabs.
(indent)
🔇 Additional comments (8)
blobl-editor/wasm/go.mod (3)
1-1: Module name change looks good.The module name has been simplified from
blobl-playgroundtoblobl, which aligns well with the other updates in this PR. The change is simple but provides better consistency across the codebase.
8-10: Explicit dependency versioning looks good.Direct dependencies on Benthos and Connect are clearly specified, which aligns with the PR objectives to automatically update only these components.
3-5:Details
❓ Verification inconclusive
Confirm Go version compatibility with your deployment environment.
The file specifies Go 1.24.1 with toolchain 1.24.2, which are very recent versions of Go. Make sure your deployment environment supports these versions.
🏁 Script executed:
#!/bin/bash # Verify Go version compatibility in deployment environments go versionLength of output: 44
Ensure Go version consistency in go.mod (blobl-editor/wasm/go.mod lines 3–5)
Your go.mod declares:
go 1.24.1 toolchain go1.24.2Local environment reports:
go version go1.24.1 linux/amd64• There’s a mismatch: the
toolchaindirective requires Go 1.24.2, but your local (and possibly CI/deploy) is on 1.24.1.
• Either upgrade all environments (CI, containers, servers) to Go 1.24.2 to match the directive,
or change thetoolchainline togo1.24.1if you intend to stay on that version..github/workflows/update-go-mod.yml (1)
43-44: Improved dependency update strategy.The workflow now explicitly updates only the Benthos and Connect packages rather than updating all dependencies recursively. This provides more controlled dependency management and aligns perfectly with the PR objectives.
src/partials/bloblang-playground.hbs (1)
314-315:Details
✅ Verification successful
Ensure consistent message handling with the backend changes.
The code now strictly uses
parsedResult.msgwithout falling back to the entireparsedResultobject. While this change aligns with backend updates for more consistent message handling, it could potentially lead to issues ifmsgis not present in certain responses.
🏁 Script executed:
#!/bin/bash # Look for places where blobl function returns values without a msg property cd blobl-editor/wasm grep -r "return" --include="*.go" . | grep -v "msg"Length of output: 1825
🏁 Script executed:
#!/bin/bash # Extract the relevant JavaScript logic around parsedResult usage sed -n '250,350p' src/partials/bloblang-playground.hbsLength of output: 3234
🏁 Script executed:
#!/bin/bash # Find and print context around payload return in WASM main.go file="blobl-editor/wasm/main.go" match=$(grep -n "return string(payload)" "$file") echo "$match" if [ -n "$match" ]; then line=${match%%:*} start=$((line-10)) end=$((line+10)) sed -n "${start},${end}p" "$file" fiLength of output: 891
No changes needed for
msghandling
The WASM backend always includes a top‐levelmsgfield (it isn’t taggedomitempty), soparsedResult.msgwill always be defined whenJSON.parse(result)succeeds. Non‐JSON outputs and errors are handled in separate branches..github/workflows/validate-build.yml (1)
16-19: Go linting integration looks good.Adding golangci-lint to validate Go code quality is a great improvement. This ensures the WebAssembly code meets quality standards before building.
blobl-editor/wasm/main.go (2)
25-32: Nice touch: converting returned errors to strings avoids JS panicsThe deferred conversion guarantees the JS caller always receives a string, simplifying the playground’s error handling. 👍
72-80: Graceful fallback from structured → raw bytes is helpfulThe extra
AsBytes()branch ensures mappings that return plain byte arrays still render correctly, matching the PR objective. Looks good.
bfd6db0 to
2b9b44a
Compare
- Only update Benthos and Connect automatically - Make sure we parse the output correctly when the mapping returns a simple byte array - Make sure we pull in the latest wasm_exec.js - Update the Readme Signed-off-by: Mihai Todor <[email protected]>
2b9b44a to
695804f
Compare
JakeSCahill
left a comment
There was a problem hiding this comment.
Amazing, thanks for the changes ❤️