Re: [lmi] Reporting errors with context (PATCH)

[Vadim Zeitlin]

On Mon, 10 Jun 2019 18:31:07 +0200 I wrote:

VZ> On Tue, 4 Jun 2019 02:16:35 +0200 I wrote:
VZ> 
VZ> VZ> On Sun, 2 Jun 2019 22:33:03 +0000 Greg Chicares <address@hidden> wrote:
VZ> VZ> 
VZ> VZ> [...we agreed on including the current page name in all exceptions 
thrown
VZ> VZ>     by pdf_writer_wx...]
[... some other ideas snipped ...]
VZ>  Thinking more about this, I have no idea what's wrong with doing it in the
VZ> following extremely simple way: just add try/catch block around the loops
VZ> iterating over the pages in pdf_command_wx.cpp and rethrow any exception
VZ> caught by it after appending the information about the page in which it
VZ> happened. This doesn't require any macros and all the changes are localized
VZ> in one place only. It also has the advantage of catching all errors, and
VZ> not only those generated by pdf_writer_wx.

 Hello again,

 To make this more concrete, here is the PR implementing the idea above:

        https://github.com/vadz/lmi/pull/116

It's really stupidly simple, especially if you look at it ignoring
whitespace, e.g. by using this URL:

        https://github.com/vadz/lmi/pull/116/files?w=1

 It might be a bit too simple because it repeats the more or less the same
catch block twice and I thought about making a function that could be
called from both places instead, but I wasn't sure if it was going to
really help so, in doubt, we decided to keep the code as it is -- but it
could be easily changed to use a helper function if you prefer, of course.

VZ>  The only possible drawback I can see is that not doing it inside
VZ> pdf_writer_wx class means that any other users of this class would need to
VZ> do something similar [...]

 The other drawback is that we assume the only exceptions thrown are those
of std::runtime_error type. This is indeed the case in practice but it's a
bit of an arbitrary limitation, of course. We could relax it by either
catching any std::exception and rethrowing it as std::runtime_error, which
would be simple enough but questionable, or doing something similar to what
catch_exceptions() does, which would add a lot of code that wouldn't be
used at all currently. Again, I'm not sure what would your preference be
here, so I preferred to just keep things simple for now, knowing that
nothing particularly catastrophic will happen even if an exception of
another type is ever thrown -- it just won't be augmented with the page
information.

 Please let me know what do you think about the current PR and, especially,
if you'd like either of the 2 aspects above to be changed.

 Thanks in advance!
VZ

pgp_eVZEPnq1x.pgp
Description: PGP signature