<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p>Sean,</p>
<p>that's a good and extensive analysis of error handling/reporting
in GDAL.<br>
</p>
<br>
<blockquote type="cite"
cite="mid:CAOodmJqF07zVA4eobrgxw+xD=1p6cxJLnMTvpXBpN1LoOex_GA@mail.gmail.com">
<div dir="ltr">
<div>Consider <span class="gmail-pl-en">OGRXLSDataSource::Open
at </span><a
href="https://github.com/OSGeo/gdal/blob/35c07b18316b4b6d238f6d60b82c31e25662ad27/ogr/ogrsf_frmts/xls/ogrxlsdatasource.cpp#L116-L118"
moz-do-not-send="true" class="moz-txt-link-freetext">https://github.com/OSGeo/gdal/blob/35c07b18316b4b6d238f6d60b82c31e25662ad27/ogr/ogrsf_frmts/xls/ogrxlsdatasource.cpp#L116-L118</a>.
The code resets the error context, pushes GDAL's silencing
handler so that no other handlers (like GDAL's default which
prints to stderr) receive error events, calls CPLRecode, and
then executes more statements if CPLRecode set an error. This
looks to me like GDAL's equivalent of what might be written in
Python as<br>
<div class="gmail-snippet-clipboard-content gmail-notranslate
gmail-position-relative gmail-overflow-auto">
<pre class="gmail-notranslate"><code>try:
CPLRecode(...)
except:
CPLGenerateTemporaryFilename(...)
...</code></pre>
</div>
</div>
</div>
</blockquote>
<code>Yes, such error is fully handled internally by GDAL and hidden
from the user. Purely implementation detail</code><br>
<blockquote type="cite"
cite="mid:CAOodmJqF07zVA4eobrgxw+xD=1p6cxJLnMTvpXBpN1LoOex_GA@mail.gmail.com">
<div dir="ltr"><br>
<div><span class="gmail-pl-en">I see different behavior when
rasterio calls GDALDatasetRasterIOEx to read data from a
GeoTIFF. The silencing handler is not used, so error events
are printed to stderr, but callers set new errors on top of
the previous ones. A rasterio users sees the deeper causes
of I/O failure in their logs, but can't react to them in
their programs without extra work to parse errors out of log
messages.</span></div>
<div><span class="gmail-pl-en"><br>
</span></div>
<div>Specifically, here's a snippet of errors printed to stderr
that was provided by a rasterio user recently. These result
from a call to <span class="gmail-pl-en">GDALDatasetRasterIOEx.</span></div>
<div>
<pre class="gmail-notranslate"><code>ERROR 1: TIFFFillTile:No space for data buffer at scanline 4294967295
ERROR 1: TIFFReadEncodedTile() failed.
ERROR 1: /home/ubuntu/Documents/CDL_tiffs/2015_30m_cdls.tif, band 1: IReadBlock failed at X offset 189, Y offset 60: TIFFReadEncodedTile() failed.
</code></pre>
"IReadBlock failed" is the last error set before <span
class="gmail-pl-en">GDALDatasetRasterIOEx returns and is the
only one that rasterio can currently surface as a Python
exception. It's specific about the block address at which a
problem occurred, but vague about the nature of the root
problem. Was it a codec error? Was it a memory allocation
error? In this case it's a memory allocation error. The user
found that they could retry data reads and get results the
next time, presumably after their program's memory footprint
shrinks sufficiently. What if we could surface enough error
detail to a user that they could determine whether they
could retry a read or not?</span></div>
</div>
</blockquote>
As mentioned in <a class="moz-txt-link-freetext" href="https://github.com/OSGeo/gdal/issues/6071">https://github.com/OSGeo/gdal/issues/6071</a>, even for
a memory allocation error, it is not obvious to know if it is makes
sense to retry because some memory allocation errors are related to
corrupted data.<br>
<blockquote type="cite"
cite="mid:CAOodmJqF07zVA4eobrgxw+xD=1p6cxJLnMTvpXBpN1LoOex_GA@mail.gmail.com">
<div dir="ltr">
<div><span class="gmail-pl-en"><br>
</span></div>
I think this could make communication in the Rasterio issue
tracker much more productive. More information about the causes
of an error is right there in the traceback instead of being
split between traceback and stderr (or other log stream). It
could at least eliminate one round of asking for more error
detail in a bug report. And there's the ability to catch an
exception and go up the chain in code, potentially very powerful
when you need it.<br>
</div>
</blockquote>
It looks to me that if GDAL's error handling could potentially be
enhanced with an error chaining concept. New API would have to be
devised (for error emitters and consumers), and relevant places
modified to use it. Would that be useful?<br>
<blockquote type="cite"
cite="mid:CAOodmJqF07zVA4eobrgxw+xD=1p6cxJLnMTvpXBpN1LoOex_GA@mail.gmail.com">
<div dir="ltr">
<div><span class="gmail-pl-en"><br>
</span></div>
<div><span class="gmail-pl-en"><span class="gmail-pl-en"> I
believe we've seen cases of GDAL functions that set errors
while returning a success error code. </span></span></div>
</div>
</blockquote>
<p>There are 2 sub cases:</p>
<p>a) the function emits a CPLError(CE_Failure, ...) , clears the
error state and returns something successful</p>
<p>b) the function emits a CPLError(CE_Failure, ...), doesn't clear
the error state and returns something successful</p>
<p>I'd say b) is probably mostly unwanted, and should be corrected
to, at the very least, clear the error state (*). But people
could also say that as soon as a CE_Failure is emitted and reaches
the error handling method (either the default one or a user-set),
that it is it is not caught by GDAL internals, the user could
assume that the global function would fail. Making such an
assumption currently given the whole code base would be overly
cautious. Worth mentioning: the CPLTurnFailureIntoWarning()
function (introduced in
<a class="moz-txt-link-freetext" href="https://github.com/OSGeo/gdal/commit/f151e9f652b9d036cacdebf67ca88f59d5680cb3">https://github.com/OSGeo/gdal/commit/f151e9f652b9d036cacdebf67ca88f59d5680cb3</a>
and from what I can see , it is pretty much only used at the 2
initial only places of gdal_misc.cpp of this commit), that as its
name suggests can cause any CE_Failure to be turned into a
CE_Warning. It could be handy to implement a "any CE_Failure that
goes to the default or user-set error handling method means that
the function failed" policy, but I'm not sure how we could
implement this consistently in the code base. That would require
analyzing all potential code paths where this could happen.
Impractical (this also applies to the (*) at the beginning of this
paragraph)<br>
</p>
<p>(I guess we all agree that a successful function may emit one or
several CPLError(CE_Warning, ...) during its execution)<br>
</p>
<blockquote type="cite"
cite="mid:CAOodmJqF07zVA4eobrgxw+xD=1p6cxJLnMTvpXBpN1LoOex_GA@mail.gmail.com">
<div dir="ltr">
<div><span class="gmail-pl-en"><span class="gmail-pl-en">It's
possible that some functions return a failed error code
while not setting any error.</span></span></div>
</div>
</blockquote>
<p>Yes, sometimes error messages aren't really helpful when it is
about corrupted data. e.g if you parse WKB geometries there are
tons of potential error situations, and emitting a specific error
for each one would require a lot of coding effort, would bloat
the code base & the size of the GDAL binary.</p>
<p>Even</p>
--
<pre class="moz-signature" cols="72"><a class="moz-txt-link-freetext" href="http://www.spatialys.com">http://www.spatialys.com</a>
My software is free, but my time generally not.</pre>
</body>
</html>