Skip to content

Commit

Permalink
fix: fix log panic (#149)
Browse files Browse the repository at this point in the history
  • Loading branch information
abuchanan-airbyte authored Nov 12, 2024
1 parent 2812ddd commit f9687e7
Show file tree
Hide file tree
Showing 5 changed files with 116 additions and 4 deletions.
7 changes: 6 additions & 1 deletion internal/cmd/local/local/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,12 @@ func (c *Command) diagnoseAirbyteChartFailure(ctx context.Context, chartErr erro
if err != nil {
msg = "unknown: failed to get pod logs."
}
pterm.Debug.Println("found logs: ", logs[0:50])

preview := logs
if len(preview) > 50 {
preview = preview[:50]
}
pterm.Debug.Println("found logs: ", preview)

trace.AttachLog(fmt.Sprintf("%s.log", pod.Name), logs)

Expand Down
63 changes: 62 additions & 1 deletion internal/cmd/local/local/install_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"helm.sh/helm/v3/pkg/chart"
"helm.sh/helm/v3/pkg/release"
"helm.sh/helm/v3/pkg/repo"
corev1 "k8s.io/api/core/v1"
)

const portTest = 9999
Expand Down Expand Up @@ -158,7 +159,6 @@ func TestCommand_Install(t *testing.T) {
return nil
}),
)

if err != nil {
t.Fatal(err)
}
Expand All @@ -172,6 +172,67 @@ func TestCommand_Install(t *testing.T) {
}
}

func TestCommand_InstallError(t *testing.T) {
testErr := errors.New("test error")
valuesYaml := mustReadFile(t, "testdata/test-edition.values.yaml")

helm := mockHelmClient{
installOrUpgradeChart: func(ctx context.Context, spec *helmclient.ChartSpec, opts *helmclient.GenericHelmOptions) (*release.Release, error) {
return nil, testErr
},
getChart: func(name string, _ *action.ChartPathOptions) (*chart.Chart, string, error) {
return &chart.Chart{Metadata: &chart.Metadata{Version: "test.airbyte.version"}}, "", nil
},
}

k8sClient := k8stest.MockClient{
FnIngressExists: func(ctx context.Context, namespace string, ingress string) bool {
return false
},
FnLogsGet: func(ctx context.Context, namespace, name string) (string, error) {
return "short", nil
},
FnPodList: func(ctx context.Context, namespace string) (*corev1.PodList, error) {
// embedded structs make it easier to set fields this way
pod := corev1.Pod{}
pod.Name = "test-pod-1"
pod.Status.Phase = corev1.PodFailed
pod.Status.Reason = "test-reason"
return &corev1.PodList{Items: []corev1.Pod{pod}}, nil
},
}

tel := telemetry.MockClient{}

httpClient := mockHTTP{do: func(req *http.Request) (*http.Response, error) {
return &http.Response{StatusCode: 200}, nil
}}
installOpts := &InstallOpts{
HelmValuesYaml: valuesYaml,
AirbyteChartLoc: testAirbyteChartLoc,
}

c, err := New(
k8s.TestProvider,
WithPortHTTP(portTest),
WithHelmClient(&helm),
WithK8sClient(&k8sClient),
WithTelemetryClient(&tel),
WithHTTPClient(&httpClient),
WithBrowserLauncher(func(url string) error {
return nil
}),
)
if err != nil {
t.Fatal(err)
}
err = c.Install(context.Background(), installOpts)
expect := "unable to install airbyte chart:\npod test-pod-1: unknown"
if expect != err.Error() {
t.Errorf("expected %q but got %q", expect, err)
}
}

func mustReadFile(t *testing.T, name string) string {
b, err := os.ReadFile(name)
if err != nil {
Expand Down
3 changes: 3 additions & 0 deletions internal/cmd/local/local/mock_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ type mockHelmClient struct {
}

func (m *mockHelmClient) AddOrUpdateChartRepo(entry repo.Entry) error {
if m.addOrUpdateChartRepo == nil {
return nil
}
return m.addOrUpdateChartRepo(entry)
}

Expand Down
17 changes: 15 additions & 2 deletions internal/telemetry/segment.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,21 +86,34 @@ func (s *SegmentClient) User() string {
return s.cfg.AnalyticsID.toUUID().String()
}

func (s *SegmentClient) Wrap(ctx context.Context, et EventType, f func() error) error {
func (s *SegmentClient) Wrap(ctx context.Context, et EventType, f func() error) (res error) {
attemptSuccessFailure := true

if err := s.Start(ctx, et); err != nil {
pterm.Debug.Printfln("Unable to send telemetry start data: %s", err)
attemptSuccessFailure = false
}

defer func() {
if r := recover(); r != nil {
pterm.Error.Printfln("recovered from panic: %v", r)
err, ok := r.(error)
if !ok {
err = fmt.Errorf("panic: %v", r)
}
res = err
if errTel := s.Failure(ctx, et, err); errTel != nil {
pterm.Debug.Printfln("Unable to send telemetry failure data: %s", errTel)
}
}
}()

if err := f(); err != nil {
if attemptSuccessFailure {
if errTel := s.Failure(ctx, et, err); errTel != nil {
pterm.Debug.Printfln("Unable to send telemetry failure data: %s", errTel)
}
}

return err
}

Expand Down
30 changes: 30 additions & 0 deletions internal/telemetry/segment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -864,6 +864,36 @@ func TestSegmentClient_WrapErr(t *testing.T) {
}
}

func TestSegmentClient_WrapPanic(t *testing.T) {
mDoer := &mockDoer{
do: func(r *http.Request) (*http.Response, error) {
return &http.Response{Body: io.NopCloser(&strings.Reader{})}, nil
},
}
opts := []Option{
WithSessionID(sessionID),
WithHTTPClient(mDoer),
}
ctx := context.Background()
cli := NewSegmentClient(Config{AnalyticsID: UUID(userID)}, opts...)
expect := errors.New("test panic err")

err := cli.Wrap(ctx, Install, func() error {
panic(expect)
})
if !errors.Is(err, expect) {
t.Errorf("expected %q but got %q", expect, err)
}

expectStr := "panic: test str"
err = cli.Wrap(ctx, Install, func() error {
panic("test str")
})
if err == nil || err.Error() != expectStr {
t.Errorf("expected %q but got %q", expectStr, err)
}
}

// --- mocks
var _ Doer = (*mockDoer)(nil)

Expand Down

0 comments on commit f9687e7

Please sign in to comment.