Skip to content

fix and check all suppressed errors#138

Draft
maxenglander wants to merge 1 commit into
maxeng-error-handling-3from
maxeng-error-handling-4
Draft

fix and check all suppressed errors#138
maxenglander wants to merge 1 commit into
maxeng-error-handling-3from
maxeng-error-handling-4

Conversation

@maxenglander

Copy link
Copy Markdown
Collaborator

Prev: #137

Fix all cases of ..., _ := errorReturningThing().

Signed-off-by: Max Englander <max@planetscale.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses the suppression of errors by replacing blank identifier patterns with proper error checking in the code.

  • Replaces error suppression with explicit error handling in the TableCursorToSerializedCursor function.
  • Implements error handling for checkConnectionStatus to log connection errors.
  • Updates .golangci.yml to enable errcheck with check-blank set to true.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
cmd/internal/planetscale_connection.go Adds error handling for converting TableCursor to a serialized cursor.
cmd/airbyte-source/check.go Implements error handling for checking the connection status.
.golangci.yml Updates linting configuration to check for blank error assignments.


cs, _ := checkConnectionStatus(ch.Database, psc)
cs, err := checkConnectionStatus(ch.Database, psc)
if err != nil {

Copilot AI Apr 3, 2025

Copy link

Choose a reason for hiding this comment

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

After logging the error in the 'if' block, consider altering the control flow to avoid using 'cs' when an error occurs. For example, returning from the function or using an alternative handling mechanism would prevent potential issues from processing an invalid connection status.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I tried doing that originally, but it broke tests. Let's add a // TODO here to fix that.

@nickvanw nickvanw left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yes

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.

3 participants