Skip to content

Conversation

CatalinSnyk
Copy link
Contributor

Pull Request Submission Checklist

  • Follows CONTRIBUTING guidelines
  • Commit messages
    are release-note ready, emphasizing
    what was changed, not how.
  • Includes detailed description of changes
  • Contains risk assessment (Low | Medium | High)
  • Highlights breaking API changes (if applicable)
  • Links to automated tests covering new functionality
  • [] Includes manual testing instructions (if necessary)
  • Updates relevant GitBook documentation (PR link: ___)
  • Includes product update to be announced in the next stable release notes

What does this PR do?

This PR handles type conversions and errors when formatting logging timestamps, returning an empty string is definitely better than crashing the CLI run.

This also lands a change from code-client-go that removes some redundant trace logs for request bodies; and also a GAF change for limiting the size of request bodies that are logged. All of those should fix some crashes when using the CLI with debug logs and trace level enabled.

Where should the reviewer start?

Main changes here are early returning in case of type conversion error when casting the interface to a string; and I also added an early return in case the time string formatting fails, just for good measure.

How should this be manually tested?

What's the product update that needs to be communicated to CLI users?

This should fix some crashes when the CLI was used with debug and trace logging level enabled.

@CatalinSnyk CatalinSnyk requested a review from a team as a code owner July 15, 2025 10:36
Copy link

snyk-io bot commented Jul 15, 2025

🎉 Snyk checks have passed. No issues have been found so far.

security/snyk check is complete. No issues have been found. (View Details)

license/snyk check is complete. No issues have been found. (View Details)

code/snyk check is complete. No issues have been found. (View Details)

cursor[bot]

This comment was marked as outdated.

@CatalinSnyk CatalinSnyk force-pushed the fix/CLI-1004_handle_logging_timestamp_formatting_errors branch from d2adbe5 to 9fe4a4f Compare July 15, 2025 10:39
@CatalinSnyk CatalinSnyk changed the title fix(logging): fix(logging): handle timestamp errors and limit request body log size Jul 15, 2025
@CatalinSnyk CatalinSnyk changed the title fix(logging): handle timestamp errors and limit request body log size fix(logging): prevent crashes when using debug and trace level logging Jul 15, 2025
@CatalinSnyk CatalinSnyk force-pushed the fix/CLI-1004_handle_logging_timestamp_formatting_errors branch from 9fe4a4f to 068ba0f Compare July 15, 2025 12:14
@CatalinSnyk CatalinSnyk force-pushed the fix/CLI-1004_handle_logging_timestamp_formatting_errors branch from 068ba0f to 92fa8be Compare July 15, 2025 12:17
@CatalinSnyk CatalinSnyk merged commit 2ea4558 into main Jul 15, 2025
9 checks passed
@CatalinSnyk CatalinSnyk deleted the fix/CLI-1004_handle_logging_timestamp_formatting_errors branch July 15, 2025 13:32
j-luong pushed a commit that referenced this pull request Jul 15, 2025
…amp_formatting_errors

fix(logging): prevent crashes when using debug and trace level logging
j-luong pushed a commit that referenced this pull request Jul 15, 2025
…amp_formatting_errors

fix(logging): prevent crashes when using debug and trace level logging
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.

2 participants