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

fix: handle case of text when invalid JSON (#3119) #3120

Conversation

TheoD02
Copy link
Contributor

@TheoD02 TheoD02 commented Sep 17, 2024

Description

This PR addresses the issue where Bruno crashes when receiving non-JSON responses (like "true") with the application/json content type. The crash was caused by Bruno attempting to parse an invalid JSON response. The fix modifies the return data handling to prevent the crash.

fix: #3119

Changes Implemented:

  1. Modified return data handling:

    • The response data is now passed through safeStringifyJSON(data) to ensure that even non-JSON responses are handled safely without causing a crash.
  2. Early return for null data:

    • Added an early return if data is null, ensuring that the logic to process the response is skipped, preventing potential errors from treating null as data.
  3. Removed unused parameter:

    • Removed the second parameter in safeStringifyJSON calls where it was set to true, since the function definition only expects one parameter.

Testing and Validation:

  • This fix has been tested to ensure that valid JSON responses are still handled correctly.
  • Cases where non-JSON responses like "true" are returned now no longer cause the client to crash.
  • Additional edge cases, such as null responses, are properly handled with the early return.

image

Contribution Checklist:

  • The pull request only addresses one issue or adds one feature.
  • The pull request does not introduce any breaking changes
  • I have added screenshots or gifs to help explain the change if applicable.*
  • I have read the contribution guidelines.
  • Create an issue and link to the pull request.

@helloanoop helloanoop merged commit 938e056 into usebruno:main Sep 18, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bruno crashes when parsing non-JSON success response
3 participants