8000 UX: better error message for json errors while parsing options by mstoykov · Pull Request #4602 · grafana/k6 · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

UX: better error message for json errors while parsing options #4602

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 12, 2025
Merged

UX: better error message for json errors while parsing options #4602

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a2da3f2
UX: better error message for json errors while parsing options
mstoykov Mar 5, 2025
43f1e7f
fixup! UX: better error message for json errors while parsing options
mstoykov Mar 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions internal/js/bundle.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@ func (b *Bundle) populateExports(updateOptions bool, bi *BundleInstance) error {
dec.DisallowUnknownFields()
if err = dec.Decode(&b.Options); err != nil {
if uerr := json.Unmarshal(data, &b.Options); uerr != nil {
uerr = beautifyJSONUnmarshalError(data, uerr)
err = errext.WithAbortReasonIfNone(
errext.WithExitCodeIfNone(uerr, exitcodes.InvalidConfig),
errext.AbortedByScriptError,
Expand Down Expand Up @@ -247,6 +248,19 @@ func (b *Bundle) populateExports(updateOptions bool, bi *BundleInstance) error {
return nil
}

func beautifyJSONUnmarshalError(data []byte, err error) error {
unmarshalTypError := new(json.UnmarshalTypeError)
if errors.As(err, &unmarshalTypError) {
e := unmarshalTypError
previousNewLineIndex := max(bytes.LastIndexByte(data[:e.Offset], '\n'), 0)
nextNewLineIndex := max(bytes.IndexByte(data[e.Offset:], '\n'), len(data)-1)

info := strings.TrimSpace(string(data[previousNewLineIndex:nextNewLineIndex]))
err = fmt.Errorf("parsing options from script got error while parsing %q: %w", info, e)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: The function name looks pretty generic, while in a large package, and while this error formatting operation is quite specific, thus I'm concerned someone could use it by mistake (expecting it to behave in a more generic way). Wdyt? Perhaps we can set a slightly more concrete name?

Copy link
Member
@inancgumus inancgumus Mar 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you don't mind longer functions, I usually find it useful to bring related functionality together. So, this function can go into a closure in the populateExport function if we want to avoid others using it as a generic function. Doing so will also save us from finding a better name for it. No strong opinion, though :)

}
return err
}

// Instantiate creates a new runtime from this bundle.
func (b *Bundle) Instantiate(ctx context.Context, vuID uint64) (*BundleInstance, error) {
// Instantiate the bundle into a new VM using a bound init context. This uses a context with a
Expand Down
17 changes: 15 additions & 2 deletions internal/js/bundle_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,11 +230,24 @@ func TestNewBundle(t *testing.T) {
invalidOptions := map[string]struct {
Expr, Error string
}{
"Array": {`[]`, "json: cannot unmarshal array into Go value of type lib.Options"},
"Function": {`function(){}`, "error parsing script options: json: unsupported type: func(sobek.FunctionCall) sobek.Value"},
"Array": {
`[]`,
`parsing options from script got error while parsing "[": ` +
`json: cannot unmarshal array into Go value of type lib.Options`,
},
"Bad value": {
`{"tags":["something"]}`,
`parsing options from script got error while parsing "{\"tags\":[\"something\"]": ` +
`json: cannot unmarshal array into Go struct field Options.tags of type map[string]string`,
},
"Function": {
`function(){}`,
"error parsing script options: json: unsupported type: func(sobek.FunctionCall) sobek.Value",
},
}
for name, data := range invalidOptions {
t.Run(name, func(t *testing.T) {
t.Parallel()
_, err := getSimpleBundle(t, "/script.js", fmt.Sprintf(`
export let options = %s;
export default function() {};
Expand Down
Loading
0