Skip to content
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

[CAPPL-324] Fix panic when fetching binary #15490

Merged
merged 6 commits into from
Dec 4, 2024

Conversation

cedric-cordenier
Copy link
Contributor

@cedric-cordenier cedric-cordenier commented Dec 3, 2024

  • Fix panic when fetching binary
  • Lazy load the connector by creating a FetcherService, and starting it after the GatewayConnector starts. This avoids GatewayConnector being nil and prevents us from panicking when calling Fetch.
  • Add some logging
  • Remove logging of the request/response body -- this was causing my local node to crash since the body was so large.

Copy link
Contributor

github-actions bot commented Dec 3, 2024

AER Report: CI Core

aer_workflow , commit , Scheduled Run Frequency , Clean Go Tidy & Generate , Detect Changes , Flakeguard Root Project / Get Tests To Run , lint , Core Tests (go_core_tests) , Flakeguard Deployment Project , Core Tests (go_core_tests_integration) , Core Tests (go_core_ccip_deployment_tests) , Core Tests (go_core_race_tests) , Core Tests (go_core_fuzz) , Flakeguard Root Project / Run Tests (github.com/smartcontractkit/chainlink/v2/core/services/relay/evm/capabilities/workflow... , Flakeguard Root Project / Run Tests (github.com/smartcontractkit/chainlink/v2/core/services/workflows/syncer, ubuntu-latest) , Flakeguard Root Project / Report , Flakey Test Detection , SonarQube Scan

1. Error running tests: failed to parse test results: failed to attribute panic to test

[A 1 <= 10 words sentence that describes the error]:[Run tests with flakeguard]

Source of Error:
Run tests with flakeguard	2024-12-04T13:54:33.3204830Z Error running tests: failed to parse test results: failed to attribute panic to test, using regex syncer\.(Test[^\.\(]+) on these strings:
Run tests with flakeguard	2024-12-04T13:54:33.3206714Z panic: Log in goroutine after Test_workflowRegisteredHandler/success_with_active_workflow_registered has completed: 2024-12-04T13:54:33.198Z	DEBUG	CapabilitiesRegistry	capabilities/registry.go:69	get capability	{"version": "unset@unset", "id": "[email protected]"}
Run tests with flakeguard	2024-12-04T13:54:33.3207635Z 
Run tests with flakeguard	2024-12-04T13:54:33.3207764Z goroutine 345 [running]:
Run tests with flakeguard	2024-12-04T13:54:33.3208185Z testing.(*common).logDepth(0xc000997a00, {0xc00127c820, 0x9b}, 0x3)
Run tests with flakeguard	2024-12-04T13:54:33.3208725Z 	/opt/hostedtoolcache/go/1.23.3/x64/src/testing/testing.go:1029 +0x6d4
Run tests with flakeguard	2024-12-04T13:54:33.3209212Z testing.(*common).log(...)
Run tests with flakeguard	2024-12-04T13:54:33.3209843Z 	/opt/hostedtoolcache/go/1.23.3/x64/src/testing/testing.go:1011
Run tests with flakeguard	2024-12-04T13:54:33.3210792Z testing.(*common).Logf(0xc000997a00, {0x3c7b312, 0x2}, {0xc00109cef0, 0x1, 0x1})
Run tests with flakeguard	2024-12-04T13:54:33.3212171Z 	/opt/hostedtoolcache/go/1.23.3/x64/src/testing/testing.go:1062 +0xa5
Run tests with flakeguard	2024-12-04T13:54:33.3213455Z go.uber.org/zap/zaptest.TestingWriter.Write({{0x7fbbaef2e6b8?, 0xc000997a00?}, 0xe0?}, {0xc000d47c00, 0x9c, 0x400})
Run tests with flakeguard	2024-12-04T13:54:33.3216334Z 	/home/runner/go/pkg/mod/go.uber.org/[email protected]/zaptest/logger.go:146 +0x11e
Run tests with flakeguard	2024-12-04T13:54:33.3217971Z go.uber.org/zap/zapcore.(*ioCore).Write(0xc000cc9d70, {0xff, {0xc1cc35a64bda5146, 0x19cb14470d, 0x5900980}, {0x3ca2f64, 0x14}, {0x3c96bdc, 0xe}, {0x1, ...}, ...}, ...)
Run tests with flakeguard	2024-12-04T13:54:33.3219370Z 	/home/runner/go/pkg/mod/go.uber.org/[email protected]/zapcore/core.go:99 +0x193
Run tests with flakeguard	2024-12-04T13:54:33.3220222Z go.uber.org/zap/zapcore.(*CheckedEntry).Write(0xc000cd56c0, {0xc001543c00, 0x1, 0x2})
Run tests with flakeguard	2024-12-04T13:54:33.3220967Z 	/home/runner/go/pkg/mod/go.uber.org/[email protected]/zapcore/entry.go:253 +0x1f0
Run tests with flakeguard	2024-12-04T13:54:33.3221875Z go.uber.org/zap.(*SugaredLogger).log(0xc00008f188, 0xff, {0x3c96bdc, 0xe}, {0x0, 0x0, 0x0}, {0xc000e03440, 0x2, 0x2})
Run tests with flakeguard	2024-12-04T13:54:33.3222936Z 	/home/runner/go/pkg/mod/go.uber.org/[email protected]/sugar.go:355 +0x130
Run tests with flakeguard	2024-12-04T13:54:33.3223732Z go.uber.org/zap.(*SugaredLogger).Debugw(...)
Run tests with flakeguard	2024-12-04T13:54:33.3224520Z 	/home/runner/go/pkg/mod/go.uber.org/[email protected]/sugar.go:251
Run tests with flakeguard	2024-12-04T13:54:33.3226000Z github.com/smartcontractkit/chainlink/v2/core/capabilities.(*Registry).Get(0xc000b42900, {0x7fbbaf383ac8?, 0x20?}, {0xc00093c558, 0x18})
Run tests with flakeguard	2024-12-04T13:54:33.3226935Z 	/home/runner/work/chainlink/chainlink/core/capabilities/registry.go:69 +0x1e2
Run tests with flakeguard	2024-12-04T13:54:33.3227955Z github.com/smartcontractkit/chainlink/v2/core/capabilities.(*Registry).GetTrigger(0xc000b42900, {0x437e770, 0xc000dba140}, {0xc00093c558, 0x18})
Run tests with flakeguard	2024-12-04T13:54:33.3228869Z 	/home/runner/work/chainlink/chainlink/core/capabilities/registry.go:80 +0x6f
Run tests with flakeguard	2024-12-04T13:54:33.3229907Z github.com/smartcontractkit/chainlink/v2/core/services/workflows.(*Engine).resolveWorkflowCapabilities(0xc0001d8008, {0x437e770, 0xc000dba140})
Run tests with flakeguard	2024-12-04T13:54:33.3230864Z 	/home/runner/work/chainlink/chainlink/core/services/workflows/engine.go:172 +0x17e
Run tests with flakeguard	2024-12-04T13:54:33.3232034Z github.com/smartcontractkit/chainlink/v2/core/services/workflows.(*Engine).init.func1()
Run tests with flakeguard	2024-12-04T13:54:33.3232765Z 	/home/runner/work/chainlink/chainlink/core/services/workflows/engine.go:322 +0x2af
Run tests with flakeguard	2024-12-04T13:54:33.3233776Z github.com/smartcontractkit/chainlink/v2/core/services/workflows.retryable({0x437e770, 0xc000dba140}, {0x43a14f0, 0xc001091020}, 0x1388, 0x0, 0xc000b81ee0)
Run tests with flakeguard	2024-12-04T13:54:33.3234659Z 	/home/runner/work/chainlink/chainlink/core/services/workflows/retry.go:45 +0x402
Run tests with flakeguard	2024-12-04T13:54:33.3235848Z github.com/smartcontractkit/chainlink/v2/core/services/workflows.(*Engine).init(0xc0001d8008, {0x437e770, 0xc000dba140})
Run tests with flakeguard	2024-12-04T13:54:33.3237271Z 	/home/runner/work/chainlink/chainlink/core/services/workflows/engine.go:313 +0x225
Run tests with flakeguard	2024-12-04T13:54:33.3239020Z created by github.com/smartcontractkit/chainlink/v2/core/services/workflows/syncer.(*eventHandler).workflowRegisteredEvent.(*Engine).Start.func1 in goroutine 33
Run tests with flakeguard	2024-12-04T13:54:33.3240593Z 	/home/runner/work/chainlink/chainlink/core/services/workflows/engine.go:153 +0x35a
Run tests with flakeguard	2024-12-04T13:54:33.3241714Z FAIL	github.com/smartcontractkit/chainlink/v2/core/services/workflows/syncer	110.832s
Run tests with flakeguard	2024-12-04T13:54:33.3242281Z 
Run tests with flakeguard	2024-12-04T13:54:33.3254134Z ##[error]Process completed with exit code 1.

Why: The error occurred because there was a panic in the test Test_workflowRegisteredHandler/success_with_active_workflow_registered after it had completed. The panic was caused by a log entry in a goroutine that continued running after the test had finished.

Suggested fix: Ensure that all goroutines are properly synchronized and terminated before the test completes. This can be done by using synchronization mechanisms like WaitGroup to wait for all goroutines to finish before ending the test.

AER Report: Operator UI CI ran successfully ✅

aer_workflow , commit

@cedric-cordenier cedric-cordenier force-pushed the CAPPL-324-panic-when-fetching-the-binary branch from 9cd062d to 428ca3c Compare December 3, 2024 13:58
@cedric-cordenier cedric-cordenier force-pushed the CAPPL-324-panic-when-fetching-the-binary branch from 428ca3c to ce1bdb8 Compare December 3, 2024 14:03
@cedric-cordenier cedric-cordenier changed the title Cappl 324 panic when fetching the binary [CAPPL-324] Fix panic when fetching binary Dec 3, 2024
- Stop logging out the request/response body
- Add a timeout when fetching the request
- Add the method in various places where it was missing
@cedric-cordenier cedric-cordenier force-pushed the CAPPL-324-panic-when-fetching-the-binary branch from 40f5583 to 9a1d94c Compare December 4, 2024 11:32
@cedric-cordenier cedric-cordenier marked this pull request as ready for review December 4, 2024 11:36
@cedric-cordenier cedric-cordenier requested review from a team as code owners December 4, 2024 11:36
ettec
ettec previously approved these changes Dec 4, 2024
Copy link
Contributor

github-actions bot commented Dec 4, 2024

Flakeguard Summary

Setting Value
Project github.com/smartcontractkit/chainlink/v2
Max Pass Ratio 100.00%
Test Run Count 6
Race Detection false
Excluded Tests TestChainComponents

Found Flaky Tests ❌

Category Total
Tests 4
Panicked Tests 1
Raced Tests 0
Flaky Tests 1
Flaky Test Ratio 25.00%
Runs 9
Passes 6
Failures 3
Skips 0
Pass Ratio 66.66%
Name Pass Ratio Panicked? Timed Out? Race? Runs Successes Failures Skips Package Package Panicked? Avg Duration Code Owners
TestNewFetcherService 0.00% true true false 3 0 3 0 github.com/smartcontractkit/chainlink/v2/core/services/workflows/syncer true 0s @smartcontractkit/keystone

@cedric-cordenier cedric-cordenier force-pushed the CAPPL-324-panic-when-fetching-the-binary branch from 4846e34 to 27b86a0 Compare December 4, 2024 13:35
@lukaszcl
Copy link
Collaborator

lukaszcl commented Dec 4, 2024

Hey @cedric-cordenier! It looks like our internal flaky test detection tool, Flakeguard, has a bug. We're working on a fix. In the meantime, you can ignore these results.

@cedric-cordenier cedric-cordenier added this pull request to the merge queue Dec 4, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 4, 2024
@jmank88 jmank88 added this pull request to the merge queue Dec 4, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 4, 2024
@jmank88 jmank88 added this pull request to the merge queue Dec 4, 2024
Merged via the queue into develop with commit 22c6cf7 Dec 4, 2024
162 of 165 checks passed
@jmank88 jmank88 deleted the CAPPL-324-panic-when-fetching-the-binary branch December 4, 2024 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants