feat: Add comprehensive IANA timezone support to Bloblang WASM playground #321
Conversation
…ound
This commit addresses a critical user issue where Bloblang's ts_tz() method failed
with IANA timezone names like 'Europe/Vilnius' in the browser-based WASM playground,
forcing users to manually calculate timezone offsets or abandon their transformations.
The core issue was that Go's time package in WASM environments lacks embedded
timezone data, causing runtime errors when users tried expressions like:
(this.StartTime / 1000).ts_tz("Europe/Vilnius").ts_strftime("%-H")
SOLUTION:
• Created JavaScript timezone bridge using Intl.DateTimeFormat API for IANA zones
• Extended Go WASM code to call out to JavaScript for timezone conversions
• Maintained backward compatibility with UTC/Local timezone handling
• Preserved full Bloblang time method chaining capabilities
TECHNICAL CHANGES:
• Modified blobl-editor/wasm/main.go to override ts_tz() method registration
• Added timezoneFuncs global object with convertToTimezone() helper
• Implemented robust fallback handling and error reporting
• Enhanced test coverage with comprehensive timezone validation
TESTING INFRASTRUCTURE:
• Built comprehensive HTML test suite for browser-based validation
• Created robust Puppeteer test runner with improved error handling
• Added CI/CD workflow for multi-version Node.js/Go compatibility testing
The solution enables users to seamlessly use any IANA timezone name in their
Bloblang transformations, eliminating the need for complex workarounds and
making the browser playground feature-complete with server-side Redpanda Connect.
All tests pass (10/10)
✅ Deploy Preview for docs-ui ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
…ersions - Update actions/upload-artifact from v3 to v4 to resolve deprecation warning - Update actions/cache from v3 to v4 for consistency and latest features - Ensures CI/CD pipeline continues working without deprecation warnings
The previous matrix was overkill for this use case: - Node.js version matters little for Puppeteer-based browser tests - Go WASM output is consistent across versions for our use case - Reduced from 7 CI runs (4 matrix + 3 compatibility) to 1 focused run Benefits: - Faster CI feedback (single run vs 7 parallel runs) - Reduced CI resource usage and costs - Simpler maintenance and debugging - Still provides adequate coverage with Node 20.x + Go 1.22.x The tests validate timezone functionality in the browser environment, which is the actual target - the build environment versions are less critical.
- Add proper error checking for fmt.Sscanf to satisfy golangci-lint errcheck - Return 0 as safe default when parsing fails - Maintains functionality while following Go best practices - All tests continue to pass (10/10)
|
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 change set introduces support for IANA timezone names in the Bloblang playground’s Sequence Diagram(s)sequenceDiagram
participant User
participant Playground UI (JS)
participant WASM Go Runtime
participant JS timezoneFuncs
User->>Playground UI (JS): Submit mapping using ts_tz with IANA timezone
Playground UI (JS)->>WASM Go Runtime: Call ts_tz(timestamp, "Europe/Vilnius")
WASM Go Runtime->>JS timezoneFuncs: convertToTimezone(timestamp, "Europe/Vilnius")
JS timezoneFuncs-->>WASM Go Runtime: Returns converted date parts or error
WASM Go Runtime->>Playground UI (JS): Returns mapped result or error
Playground UI (JS)->>User: Display output
Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
package.json (1)
11-11: Update Node.js engine requirement to match READMEThe
engines.nodefield still specifies ">= 16.0.0" while the README was updated to require Node.js 18. This inconsistency could cause confusion.Update the engine requirement:
"engines": { - "node": ">= 16.0.0" + "node": ">= 18.0.0" },
🧹 Nitpick comments (5)
gulpfile.js (2)
195-210: Test task implementation looks good with room for improvementThe test task correctly uses
spawnwithstdio: 'inherit'for proper output handling. Consider adding more detailed error reporting.Consider enhancing error handling:
const testTask = createTask({ name: 'test', desc: 'Run Bloblang playground tests (build WASM + run tests)', call: (done) => { const { spawn } = require('child_process') - const testProcess = spawn('node', ['tests/bloblang-playground/test-runner.js'], { stdio: 'inherit' }) + const testScript = path.join(__dirname, 'tests', 'bloblang-playground', 'test-runner.js') + const testProcess = spawn('node', [testScript], { stdio: 'inherit' }) + + testProcess.on('error', (err) => { + log.error('Failed to start test process:', err.message) + done(err) + }) + testProcess.on('close', (code) => { if (code === 0) { log.info('✅ All tests passed!') done() } else { done(new Error(`Tests failed with exit code ${code}`)) } }) }, })
212-222: Test task descriptions could be clearerThe descriptions for
testandtest:buildtasks are somewhat confusing. Thetesttask description mentions "build WASM + run tests" but it only runs tests.Update task descriptions for clarity:
const testTask = createTask({ name: 'test', - desc: 'Run Bloblang playground tests (build WASM + run tests)', + desc: 'Run Bloblang playground tests only', call: (done) => {package.json (1)
78-83: Consider using cross-platform path handling in scriptsThe build:wasm script uses Unix-style paths and commands that may not work on Windows. Consider using a more portable approach.
For better cross-platform compatibility, consider using a Node.js script or a build tool:
- "build:wasm": "cd blobl-editor/wasm && GOOS=js GOARCH=wasm go build -o ../../src/static/blobl.wasm .", + "build:wasm": "node scripts/build-wasm.js",Create a
scripts/build-wasm.jsfile that handles paths and environment variables in a cross-platform way.tests/bloblang-playground/bloblang-playground-test.html (1)
376-391: Improve WASM loading error handling.The error handling for WASM loading is good, but consider adding more specific error messages for common failure scenarios (network issues, WASM compilation errors, etc.).
.catch(error => { - document.getElementById('loading').innerHTML = ` - <h3>❌ Failed to load WebAssembly module</h3> - <pre style="color: red; background: #ffe6e6; padding: 10px; border-radius: 5px;">${error}</pre> - `; + const errorType = error.name === 'TypeError' ? 'Network or file not found' : + error.name === 'CompileError' ? 'WASM compilation failed' : 'Unknown error'; + document.getElementById('loading').innerHTML = ` + <h3>❌ Failed to load WebAssembly module</h3> + <p><strong>Error type:</strong> ${errorType}</p> + <pre style="color: red; background: #ffe6e6; padding: 10px; border-radius: 5px;">${error}</pre> + `;tests/bloblang-playground/test-runner.js (1)
34-50: Consider adding memory management flags for WASM testing.The browser launch configuration is conservative, but for WASM testing with large files (60MB mentioned), consider adding memory-related flags.
args: [ '--no-sandbox', '--disable-setuid-sandbox', '--disable-dev-shm-usage', + '--max-old-space-size=4096', + '--js-flags="--max-old-space-size=4096"', '--disable-extensions',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
.github/workflows/test-bloblang-playground.yml(1 hunks).gitignore(1 hunks)README.adoc(1 hunks)blobl-editor/wasm/main.go(9 hunks)gulpfile.js(2 hunks)package.json(2 hunks)tests/bloblang-playground/bloblang-playground-test.html(1 hunks)tests/bloblang-playground/test-runner.js(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
gulpfile.js (1)
gulp.d/tasks/bundle-react.js (1)
log(6-6)
🪛 actionlint (1.7.7)
.github/workflows/test-bloblang-playground.yml
32-32: 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)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Redirect rules - docs-ui
- GitHub Check: Header rules - docs-ui
- GitHub Check: Pages changed - docs-ui
- GitHub Check: build
- GitHub Check: test-bloblang-playground
🔇 Additional comments (11)
.gitignore (1)
7-7: LGTM!The addition of
test-results.*pattern appropriately excludes test result artifacts from version control.blobl-editor/wasm/main.go (1)
136-212: Minor formatting improvements look goodThe removal of backticks from error messages and the consistent use of escaped double-quoted strings improve code consistency.
README.adoc (1)
68-68: Node.js version requirement update is appropriateThe update from Node.js 16 to 18 aligns with the CI/CD workflow configuration and modern JavaScript features used in the testing infrastructure.
package.json (1)
64-65: Dependency versions are appropriateThe added Puppeteer (21.0.0) and Express (4.18.0) versions are stable releases suitable for the testing infrastructure.
tests/bloblang-playground/bloblang-playground-test.html (1)
128-159: LGTM! Well-designed timezone conversion bridge.The
timezoneFuncs.convertToTimezonefunction provides a clean JavaScript bridge for IANA timezone conversions using the Intl.DateTimeFormat API. The implementation correctly handles timestamp conversion, timezone formatting, and error cases..github/workflows/test-bloblang-playground.yml (2)
5-18: LGTM! Efficient path-based triggering.The path filters are well-designed to trigger only on relevant changes, reducing unnecessary CI runs while ensuring all critical files are monitored.
45-49: Verified:test:buildtask is defined in gulpfile.jsThe
gulpfile.jsdefines atest:buildtask via:const testBuildTask = createTask({ name: 'test:build', desc: 'Build WASM and run playground tests', call: series(buildWasmTask, testTask), })No changes are required to the workflow.
tests/bloblang-playground/test-runner.js (4)
20-30: LGTM! Robust server startup with timeout handling.The server setup uses dynamic port allocation and includes a timeout for startup, which prevents hanging on server initialization failures.
112-140: Excellent progressive timeout and fallback handling.The WASM initialization logic with progressive timeouts and fallback detection is well-designed. It handles the realistic scenario where WASM loading might timeout but tests could still proceed.
186-189: Good validation of test execution.The check for
results.total === 0properly validates that tests actually ran, preventing false positive success reports.
238-258: Excellent cleanup handling with error resilience.The cleanup logic in the finally block properly handles both browser and server cleanup, with individual error handling to prevent cleanup failures from masking the original error.
The production playground template was missing the timezoneFuncs JavaScript
helper that enables IANA timezone support in the WASM environment.
This fixes the error: 'timezone functions not available in JavaScript environment'
when users try expressions like:
(this.StartTime / 1000).ts_tz("Europe/Vilnius").ts_strftime("%-H")
Changes:
• Added window.timezoneFuncs global object with convertToTimezone method
• Uses browser's Intl.DateTimeFormat API for robust timezone conversion
• Placed before WASM loading to ensure availability when Go code initializes
• Matches the implementation used in our comprehensive test suite
Now users can seamlessly use any IANA timezone name in the production
playground, making the browser experience consistent with server-side Benthos.
|
This is hard to test. Timezones are a nightmare. The tool that I trust the most is https://www.epochconverter.com/ The current PR is failing the conversion sometimes. Suggested test to capture errorsShould produce But it's producing Tests that are workingInvalid timezones and blank work as expected, giving an error. Challenging valuesNot sure how accurate do we want to be, but there are some challenging values that we could test. DST TransitionShould be March 10, 2024 - US DST transition Year boundaryShould be December 31, 2024 23:00 UTC |
…rage 🔧 Critical Bug Fixes: - Fix ts_strftime default timezone behavior in WASM to use UTC instead of local time - Fix JavaScript Intl.DateTimeFormat hour 24 normalization for midnight edge cases - Resolve +1 hour offset bug affecting all timezone conversions - Use tagged switch for timezone handling to satisfy golangci-lint 🧪 Test Coverage Enhancements: - Add epoch timestamp (0) test to catch timezone conversion regressions - Add DST transition test for March 10, 2024 edge case - Add year boundary test with Pacific/Kiritimati extreme offset (+14 UTC) - Add direct ts_strftime vs ts_tz comparison tests - Add comprehensive timezone comparison test with multiple timezones - Improve test runner to capture detailed failure information ✅ Validation: - All 17 tests now pass (100% success rate) - Both ts_tz + ts_strftime and direct ts_strftime patterns work correctly - IANA timezone support robust for all edge cases including midnight - DST transitions handled automatically via JavaScript Intl.DateTimeFormat The fix ensures that timestamp 0 correctly produces: - UTC: 0 (midnight Jan 1, 1970) - America/New_York: 19 (7 PM Dec 31, 1969) - Asia/Tokyo: 9 (9 AM Jan 1, 1970) Addresses timezone conversion issues reported in production usage.
This PR addresses a critical user issue where Bloblang's ts_tz() method failed with IANA timezone names like 'Europe/Vilnius' in the browser-based WASM playground, forcing users to manually calculate timezone offsets or abandon their transformations.
The core issue was that Go's time package in WASM environments lacks embedded timezone data, causing runtime errors when users tried expressions like:
(this.StartTime / 1000).ts_tz("Europe/Vilnius").ts_strftime("%-H")
SOLUTION:
• Created JavaScript timezone bridge using Intl.DateTimeFormat API for IANA zones • Extended Go WASM code to call out to JavaScript for timezone conversions • Maintained backward compatibility with UTC/Local timezone handling • Preserved full Bloblang time method chaining capabilities
TECHNICAL CHANGES:
• Modified blobl-editor/wasm/main.go to override ts_tz() method registration • Added timezoneFuncs global object with convertToTimezone() helper • Implemented robust fallback handling and error reporting • Enhanced test coverage with comprehensive timezone validation
TESTING INFRASTRUCTURE:
• Built comprehensive HTML test suite for browser-based validation • Created robust Puppeteer test runner with improved error handling • Added CI/CD workflow for multi-version Node.js/Go compatibility testing
The solution enables users to seamlessly use any IANA timezone name in their Bloblang transformations, eliminating the need for complex workarounds and making the browser playground feature-complete with server-side Redpanda Connect.
All tests pass (10/10)
Test
https://deploy-preview-321--docs-ui.netlify.app/playground?input=eyAiU3RhcnRUaW1lIjogMTc1NDIyOTAwMDAwMCB9&meta=e30%3D&map=bGV0IHR6ID0gIkV1cm9wZS9WaWxuaXVzIi5vcigiVVRDIikKbGV0IHRpbWUgPSAodGhpcy5TdGFydFRpbWUgLyAxMDAwKS50c190eigiRXVyb3BlL1ZpbG5pdXMiKQpyb290LlN0YXJ0SG91ciA9ICR0aW1lLnRzX3N0cmZ0aW1lKCIlLUgiKQpyb290LnR6ID0gJHR6