Skip to content
This repository has been archived by the owner on Sep 13, 2022. It is now read-only.

Report HTTP errors when flushing spans #459

Merged
merged 2 commits into from
Apr 28, 2021

Conversation

rexxars
Copy link
Contributor

@rexxars rexxars commented Apr 6, 2021

Which problem is this PR solving?

Fixes the issue where the server might respond with an error (4xx, 5xx) but no feedback is given to the developer that can help them diagnose the issue.

Resolves #458

Short description of the changes

  • Uses the same pattern as on socket errors: builds an error message and calls the logger.error() method, as well as providing the error to the flush callback function.

Signed-off-by: Espen Hovlandsdal <espen@hovlandsdal.com>
@rexxars rexxars force-pushed the feat/log-http-report-errors branch from 8df9c0b to bca90f2 Compare April 6, 2021 04:22
@codecov
Copy link

codecov bot commented Apr 6, 2021

Codecov Report

Merging #459 (ef998ae) into master (6a6e3ea) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #459   +/-   ##
=======================================
  Coverage   98.72%   98.72%           
=======================================
  Files          50       50           
  Lines        2035     2039    +4     
  Branches      383      384    +1     
=======================================
+ Hits         2009     2013    +4     
  Misses         26       26           
Impacted Files Coverage Δ
src/reporters/http_sender.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6a6e3ea...ef998ae. Read the comment docs.

Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

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

Thanks!

@yurishkuro yurishkuro merged commit 1e55e22 into jaegertracing:master Apr 28, 2021
@rexxars rexxars deleted the feat/log-http-report-errors branch April 29, 2021 01:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HTTP errors on flush are not reported
2 participants