Skip to content
This repository was archived by the owner on Aug 30, 2022. It is now read-only.

(bugfix) Unhandled exception - Abort raised when write to the jaeger-… #80

Merged
merged 5 commits into from
Apr 16, 2018
Merged

(bugfix) Unhandled exception - Abort raised when write to the jaeger-… #80

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8f8840c
(bugfix) Unhandled exception - Abort raised when write to the jaeger-…
ankit409 Apr 8, 2018
57a6b4b
(bugfix) review comments - application crashing when agent unavailable
ankit409 Apr 13, 2018
25228fe
Add test case for new exception handling code
isaachier Apr 16, 2018
83f2605
Add coverage for unknown exceptions
isaachier Apr 16, 2018
855cddb
Merge pull request #1 from isaachier/ankit-varma10-master
ankit-varma10 Apr 16, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion src/jaegertracing/UDPTransport.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,18 @@ int UDPTransport::flush()
thrift::Batch batch;
batch.__set_process(_process);
batch.__set_spans(_spanBuffer);
_client->emitBatch(batch);

try {
_client->emitBatch(batch);
} catch (const std::logic_error &ex) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why only logic_error? I'd move this block to the end and replace logic_error with exception. That way it can be a fallback for any exception other than system_error.

Copy link
Contributor Author

@ankit-varma10 ankit-varma10 Apr 11, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Hey,

Sorry, was caught up in a bunch of other things.

"Why only logic error?"
The only two exceptions I see being raised from UDPClient::emitBatch were logic_error (packet size too big for a single UDP packet) and std::system_error. So, I think I could add one more handler to it for std:: exception.

catch (logic) {
}
catch (system) {

} catch (std:exception) {

}

Thoughts?

Copy link
Contributor

@isaachier isaachier Apr 11, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

OK cool. My idea what just skip the catch (logic) {} and do the rest as above. logic_error is derived from exception so a final block catches both.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yeah. I missed that.

Makes sense. I'll make the changes today.

std::ostringstream oss;
oss << "Could not send span " << ex.what();
throw Transport::Exception(oss.str(), _spanBuffer.size());
} catch (const std::system_error& ex) {
std::ostringstream oss;
oss << "Could not send span " << ex.what() << ", code=" << ex.code().value();
throw Transport::Exception(oss.str(), _spanBuffer.size());
}

resetBuffers();

Expand Down