Skip to content

feat: Better error when panic happens in UnifiedDiff #2217

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.

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 1 commit into from
Jul 8, 2025

Conversation

erezrokah
Copy link
Member

Summary

We have a test in our ClickHouse destination that is flaky https://github.com/cloudquery/cloudquery/actions/runs/15976814323/job/45061355720#step:9:16

FAIL: TestPlugin/TestInsert/All (2.80s)
panic: json: error calling MarshalJSON for type *array.Struct: json: error calling MarshalJSON for type json.RawMessage: unexpected character '' [recovered]
	panic: json: error calling MarshalJSON for type *array.Struct: json: error calling MarshalJSON for type json.RawMessage: unexpected character ''

goroutine 74 [running]:
testing.tRunner.func1.2({0x10f42e0, 0xc00104ccf0})
	/opt/hostedtoolcache/go/1.24.3/x64/src/testing/testing.go:1734 +0x3eb
testing.tRunner.func1()
	/opt/hostedtoolcache/go/1.24.3/x64/src/testing/testing.go:1737 +0x696
panic({0x10f42e0?, 0xc00104ccf0?})
	/opt/hostedtoolcache/go/1.24.3/x64/src/runtime/panic.go:792 +0x132
github.com/apache/arrow-go/v18/arrow/array.(*List).GetOneForMarshal(0xc00140be00, 0x0)
	/home/runner/go/pkg/mod/github.com/apache/arrow-go/v18@v18.3.1/arrow/array/list.go:110 +0x33b
github.com/apache/arrow-go/v18/arrow/array.(*Struct).GetOneForMarshal(0xc000737b00, 0x0)
	/home/runner/go/pkg/mod/github.com/apache/arrow-go/v18@v18.3.1/arrow/array/struct.go:212 +0x2d6
github.com/apache/arrow-go/v18/arrow/array.(*Struct).MarshalJSON(0xc000737b00)
	/home/runner/go/pkg/mod/github.com/apache/arrow-go/v18@v18.3.1/arrow/array/struct.go:226 +0x189
github.com/goccy/go-json/internal/encoder.AppendMarshalJSON(0xc000179110, 0xc0009b9860, {0xc000ffdc00, 0x0, 0x400}, {0x119ea80, 0xc000737b00})
	/home/runner/go/pkg/mod/github.com/goccy/go-json@v0.10.5/internal/encoder/encoder.go:435 +0x472
github.com/goccy/go-json/internal/encoder/vm.appendMarshalJSON(0xc000179110, 0xc0009b9860, {0xc000ffdc00, 0x0, 0x400}, {0x119ea80, 0xc000737b00})
	/home/runner/go/pkg/mod/github.com/goccy/go-json@v0.10.5/internal/encoder/vm/util.go:152 +0x85
github.com/goccy/go-json/internal/encoder/vm.Run(0xc000179110, {0xc000ffdc00, 0x0, 0x400}, 0xc000df6cb0)
	/home/runner/go/pkg/mod/github.com/goccy/go-json@v0.10.5/internal/encoder/vm/vm.go:271 +0x63c5
github.com/goccy/go-json.encodeRunCode(0xc000179110, {0xc000ffdc00, 0x0, 0x400}, 0xc000df6cb0)
	/home/runner/go/pkg/mod/github.com/goccy/go-json@v0.10.5/encode.go:310 +0x235
github.com/goccy/go-json.encode(0xc000179110, {0x119ea80, 0xc000737b00})
	/home/runner/go/pkg/mod/github.com/goccy/go-json@v0.10.5/encode.go:235 +0x47d
github.com/goccy/go-json.marshal({0x119ea80, 0xc000737b00}, {0x0, 0x0, 0x3?})
	/home/runner/go/pkg/mod/github.com/goccy/go-json@v0.10.5/encode.go:150 +0x16a
github.com/goccy/go-json.MarshalWithOption(...)
	/home/runner/go/pkg/mod/github.com/goccy/go-json@v0.10.5/json.go:185
github.com/goccy/go-json.Marshal({0x119ea80, 0xc000737b00})
	/home/runner/go/pkg/mod/github.com/goccy/go-json@v0.10.5/json.go:170 +0x3b
github.com/apache/arrow-go/v18/internal/json.Marshal(...)
	/home/runner/go/pkg/mod/github.com/apache/arrow-go/v18@v18.3.1/internal/json/json.go:38
github.com/apache/arrow-go/v18/arrow/array.(*List).GetOneForMarshal(0xc00140b2c0, 0x0)
	/home/runner/go/pkg/mod/github.com/apache/arrow-go/v18@v18.3.1/arrow/array/list.go:108 +0x26c
github.com/apache/arrow-go/v18/arrow/array.(*List).MarshalJSON(0xc00140b2c0)
	/home/runner/go/pkg/mod/github.com/apache/arrow-go/v18@v18.3.1/arrow/array/list.go:124 +0x189
github.com/apache/arrow-go/v18/arrow/array.stringAt({0x140ee40, 0xc00104d830}, 0x2)
	/home/runner/go/pkg/mod/github.com/apache/arrow-go/v18@v18.3.1/arrow/array/diff.go:110 +0xbdb
github.com/apache/arrow-go/v18/arrow/array.Edits.UnifiedDiff({0xc000516000, 0x5, 0x140ee40?}, {0x140ee40, 0xc00104d830}, {0x140ee40, 0xc00104d800})
	/home/runner/go/pkg/mod/github.com/apache/arrow-go/v18@v18.3.1/arrow/array/diff.go:72 +0x558
github.com/cloudquery/plugin-sdk/v4/plugin.TableDiff({0x1409f58, 0xc000298000}, {0x1409f58, 0xc000298030})
	/home/runner/go/pkg/mod/github.com/cloudquery/plugin-sdk/v4@v4.84.2/plugin/diff.go:42 +0x5da

It's hard to debug the test since:

  1. It's flaky
  2. We don't have enough information when it fails

This PR tries to error out with a bit more information on the data that was used while triggering the panic


Use the following steps to ensure your PR is ready to be reviewed

  • Read the contribution guidelines 🧑‍🎓
  • Run go fmt to format your code 🖊
  • Lint your changes via golangci-lint run 🚨 (install golangci-lint here)
  • Update or add tests 🧪
  • Ensure the status checks below are successful ✅

@erezrokah erezrokah requested a review from a team as a code owner July 7, 2025 17:20
@erezrokah erezrokah requested a review from murarustefaan July 7, 2025 17:20
@github-actions github-actions bot added the feat label Jul 7, 2025
@kodiakhq kodiakhq bot merged commit a282ce6 into main Jul 8, 2025
12 checks passed
@kodiakhq kodiakhq bot deleted the test/catch_panic_log_records branch July 8, 2025 13:32
kodiakhq bot pushed a commit that referenced this pull request Jul 8, 2025
🤖 I have created a release *beep* *boop*
---


## [4.87.0](v4.86.2...v4.87.0) (2025-07-08)


### Features

* Better error when panic happens in `UnifiedDiff` ([#2217](#2217)) ([a282ce6](a282ce6))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
kodiakhq bot pushed a commit that referenced this pull request Jul 9, 2025

Follow up to #2217
Apparently `ValueStr` calls the same code that generated the original panic. Hopefully this works

---
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants