Project

General

Profile

Actions

Task #2997

closed

Enhance exception throwing with Boost Exception library

Added by Junxiao Shi almost 9 years ago. Updated over 8 years ago.

Status:
Closed
Priority:
Normal
Category:
Base
Target version:
Start date:
Due date:
% Done:

100%

Estimated time:
3.00 h

Description

Replace all throw exceptions with BOOST_THROW_EXCEPTION macros.

This annotates the exception with diagnostic information, so that an application using ndn-cxx library can access them for easier debugging.


Related issues 1 (0 open1 closed)

Related to NFD - Task #2541: Enhance exception throwing and fatal log with Boost Exception libraryClosedSpyros Mastorakis

Actions
Actions #1

Updated by Junxiao Shi almost 9 years ago

  • Related to Task #2541: Enhance exception throwing and fatal log with Boost Exception library added
Actions #2

Updated by Spyros Mastorakis almost 9 years ago

  • Status changed from New to In Progress
  • Assignee set to Spyros Mastorakis
  • % Done changed from 0 to 10

I am uploading an initial commit in gerrit with a single Error statement changed. Please let me know if you like this structure or I should change it:

http://gerrit.named-data.net/#/c/2272/

Actions #3

Updated by Alex Afanasyev over 8 years ago

I haven't yet seen the diagnostic message output and don't remember how it looked like. But I think it would be rather verbose and not entirely user friendly. Should we enable it only in certain circumstance (like when compiled in debug mode)?

Actions #4

Updated by Davide Pesavento over 8 years ago

Crashing is never user friendly anyway. But if it's a real bug and the user opens a ticket with the extended error message, it'll be easier for us, and better than asking to (re)build in debug mode and try to reproduce the crash. So I think we should always print the extended info.

Actions #5

Updated by Spyros Mastorakis over 8 years ago

  • Status changed from In Progress to Code review
  • % Done changed from 10 to 90
Actions #6

Updated by Junxiao Shi over 8 years ago

There is a dispute on whether to print diagnostic information in ndnsec top-level catch.

ndnsec top-level catch is serving dual purposes:

  • Catch and print a legitimate error, such as "Key not found". In this case, it's better to print the error message e.what() only without diagnostics.
  • Catch unexpected error, such as TLV decoding errors. In this case, diagnostics are useful.

We should distinguish between these two purposes (using two or more separate catch branches with different types), and skip diagnostics for all legitimate errors.

Actions #7

Updated by Davide Pesavento over 8 years ago

Junxiao Shi wrote:

ndnsec top-level catch is serving dual purposes:

  • Catch and print a legitimate error, such as "Key not found". In this case, it's better to print the error message e.what() only without diagnostics.

This is abusing exceptions. "key not found" is not an exceptional case. In such case the program knows exactly what's wrong and how to solve it (exit with a clear error for the user). So you simply shouldn't use exceptions for this case.

  • Catch unexpected error, such as TLV decoding errors. In this case, diagnostics are useful.

We should distinguish between these two purposes (using two or more separate catch branches with different types), and skip diagnostics for all legitimate errors.

Don't use exceptions for non-exceptional cases.

Actions #8

Updated by Junxiao Shi over 8 years ago

I agree with note-7.

I recommend this to proceed as follows:

  • Don't add diagnostics in ndnsec.
  • Create a separate issue to avoid exceptions in non-exceptional cases; ndnsec top-level catch should add diagnostics in that issue.
Actions #9

Updated by Spyros Mastorakis over 8 years ago

Is there a consensus on how we should proceed with this issue?

Actions #10

Updated by Alex Afanasyev over 8 years ago

I agree with note 8 only partially. I would revert changes in ndnsec tools, but I don't agree with other changes. Exception that are throws from security classes are exceptional cases and need to be reported to users. ndnsec tools are user facing tools and users just need a high-level error (we may need to update error messages to make errors more user friendly in some cases).

We can also revert any changes in other tools, as they will be removed from the library anyways.

For NFD (I think it should be just nfd, not other tools), we need to agree on a reasonable format of the error message. I would like to see just content of what(), file name and line number of the place the error was thrown (if available).

Actions #11

Updated by Junxiao Shi over 8 years ago

  • Status changed from Code review to Closed
  • % Done changed from 90 to 100
Actions

Also available in: Atom PDF