Fix critical bugs, improve safety, and add tests/CI - #11
Merged
Merged
Conversation
- Fix OnNewPool dispatching to "onConfigLoaded" instead of "onNewPool" (copy-paste bug) - Fix OnTraffic swallowing error by returning nil instead of err - Add sync.Mutex to protect Goja VM from concurrent access (not goroutine-safe) - Use checked type assertion in RunFunction to prevent panics on unexpected JS return types - Fix hardcoded "OnTrafficFromClient" in RunFunction error log to use actual function name
- Add argument bounds checking to btoa, atob, and parseSQL to prevent panics when called with no arguments from JS - Handle errors from base64 decode and SQL parse instead of silently discarding them; throw JS TypeErrors on invalid input - Switch btoa/atob from RawStdEncoding to StdEncoding for browser compatibility (standard base64 with padding) - Replace deprecated golang.org/x/exp/maps with stdlib maps/slices
- Fix default metrics socket path from gatewayd-plugin-cache.sock to gatewayd-plugin-js.sock - Update stale "Template plugin" description to reflect actual purpose
- actions/checkout v3 -> v4 - actions/setup-go v3 -> v5 - softprops/action-gh-release v1 -> v2
- Add test.yaml workflow that runs on push/PR with golangci-lint, govulncheck, go test with coverage, and coveralls reporting - Add .golangci.yaml configuration matching the cache plugin's style, with depguard rules for this project's dependencies
- Test RegisterFunction/RegisterFunctions with valid and missing JS functions - Test RunFunction for success, not-found, JS error, and wrong return type - Test GetHooks returns only registered (non-nil) hooks - Test GetPluginConfig returns correct metadata - Test all 21 hook methods pass through correctly without JS functions - Test all 21 hook methods dispatch to the correct JS function name (prevents copy-paste bugs like the OnNewPool fix) - Test NewJSPlugin constructor
mostafa
changed the title
- Move context.Context to first parameter in RunFunction (revive) - Rename unused parameters to _ in GRPCServer, GRPCClient, GetPluginConfig (revive) - Define static ErrUnexpectedReturnType and wrap with fmt.Errorf (err113) - Fix gofumpt formatting in OnTrafficFromServer and OnTrafficToClient - Change NewJSPlugin to take *Plugin to avoid copying sync.Mutex (govet/copylocks) - Extract setupHelpers to reduce nesting complexity in main() (nestif) - Rename short parameter name vm -> runtime (varnamelen)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ticket(s)
N/A - issues found via code review.
Description
Critical bug fixes
OnNewPooldispatching to"onConfigLoaded"instead of"onNewPool"(copy-paste bug)OnTrafficswallowing error (returningnilinstead oferr)sync.Mutexto protect Goja VM from concurrent access (not goroutine-safe)RunFunctionto prevent panics on unexpected JS return types"OnTrafficFromClient"inRunFunctionerror log to use actual function nameHelper function improvements
btoa,atob, andparseSQL(prevents panic on no-arg calls)btoa/atobfromRawStdEncodingtoStdEncodingfor browser compatibilitygolang.org/x/exp/mapswith stdlibmaps/slicesCopy-paste artifact cleanup
gatewayd-plugin-cache.socktogatewayd-plugin-js.sockCI/CD
.golangci.yamlconfigurationTests
RunFunctionpaths,GetHooks,GetPluginConfig, all 21 hook methods for passthrough and correct dispatchRelated PRs
None.
Development Checklist
Legal Checklist