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

Enhance Loki Transport URL Handling and Error Management #170

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

sweep-ai[bot]
Copy link
Contributor

@sweep-ai sweep-ai bot commented Dec 27, 2024

Purpose

Improve the winston-loki library's robustness by implementing more comprehensive URL validation, error handling, and providing clearer error messages during Loki log transmission.

This pull request was created to solve the following GitHub issue:

Improve errors handling

Details

By the docs, the user needs to pass only host as an url option, so the exact URL is being built internally here:

https://github.com/JaniAnttonen/winston-loki/blob/development/src/batcher.js#L42-L43

But if the user doesn't follow docs, or makes a typo, and passes not only a host but for example, a full URL, then internal implementation doesn't care about it and builds an invalid URL. Then, in the req.post, an invalid URL is being silently processed with no error, so in fact, no logs are being sent, and no error is being thrown. Here's the problem:

https://github.com/JaniAnttonen/winston-loki/blob/development/src/batcher.js#L257-L272

Inside then, the response argument should be handled, and if something bad happens, it should throw an error so that catch can handle it correctly. For example, for an invalid URL the response will have a text error saying 404 URL not found.

Branch

No response


Improve Error Handling for Invalid URLs in Winston Loki Transport

Description:

Enhance error handling in the Batcher class to properly handle and report invalid URLs and HTTP response errors, particularly when users provide incorrect URL formats.

Tasks:

  1. In src/batcher.js, update URL handling:

    • Add validation for the host parameter in constructor
    • Ensure URL construction handles both host-only and full URL inputs
    • Add error handling for malformed URLs
  2. In src/batcher.js, enhance sendBatchToLoki method:

    • Add response status code validation
    • Throw appropriate errors for non-200 responses
    • Include response body in error messages

Test:

  1. In test/batcher.json.test.js, add tests:

    • Invalid host parameter
    • Full URL in host parameter
    • Non-200 HTTP responses
    • Response error messages
  2. In test/requests.test.js, add tests:

    • HTTP error responses
    • Invalid URL handling
    • Response body parsing

Additional Context:

The changes focus on the URL construction in the Batcher constructor and HTTP response handling in the sendBatchToLoki method to prevent silent failures when logs aren't being sent.

Description

This pull request introduces several key improvements to the Loki transport mechanism:

  1. URL Handling:

    • Added validation for the host parameter
    • Support for full URLs in the host configuration
    • Automatic URL parsing and normalization
    • Graceful handling of different URL formats
  2. Error Management:

    • Enhanced error detection for Loki API responses
    • More informative error messages
    • Improved error handling for connection and API-related issues
    • Preservation of original error context
  3. Request Handling:

    • Added HTTP status code validation
    • Improved error reporting for network requests
    • More robust error rejection mechanism

Summary

  • Improved URL validation in src/batcher.js
  • Enhanced error handling for Loki API interactions
  • Added comprehensive test cases for URL and error scenarios
  • Updated request handling in src/requests.js
  • Introduced more descriptive error messages
  • Validated host parameter requirements

Fixes

Fixes #169. Continue the conversation here: https://app.sweep.dev/c/c690ad31-7eb2-46c6-8848-a73d0bf83ad9.

To have Sweep make further changes, please add a comment to this PR starting with "Sweep:".

📖 For more information on how to use Sweep, please read our documentation.

Tracking ID: 3194344b75

Copy link
Contributor Author

sweep-ai bot commented Dec 27, 2024

No CI runs were found. Sweep can fix CI failures, so we recommend setting it up for your repository to let Sweep run your build and tests.

If you think this is an error, check Sweep's permissions at https://github.com/organizations/JaniAnttonen/settings/installations/49023423.

📖 For more information on how to use Sweep, please read our documentation.

Tracking ID: 3194344b75-ci-0

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.

Sweep: Improve errors handling
0 participants