Repository navigation
Report container nesting past the recursion limit as IonException - #448
Conversation
| # stream directly. A caller advancing it past the recursion limit may see a RecursionError | ||
| # instead of an IonException, depending on which depth limit the runtime reaches first. |
There was a problem hiding this comment.
Is the comment accurate? Aren't we ensuring IonException is thrown?
There was a problem hiding this comment.
Yes, it's accurate — this is the one remaining path that surfaces a bare
RecursionError. I checked all 12 public entry points across 3.9–3.13; the other
11 raise IonException everywhere. (This one does too on 3.12+, but only
incidentally: ion-c's max_container_depth is reached first there and raises from
the C side.)
The reason it's uncovered is that this branch hands back the raw C iterator, and
the caller drives it with their own next() calls, outside anything
_translate_recursion_error can wrap. I did try wrapping it in a generator, but
that breaks ionc_write's fast path for re-serializing a stream — it type-checks
for the raw iterator — which showed up as a test_ion_stream failure.
Covering it would mean teaching ionc_write to accept a wrapped iterator as well,
so it's contained but touches the write path for the sake of one lazy read entry
point. Do you want me to close the gap?
2 participants
Issue #, if available:
Description of changes:
Deeply nested containers raised
RecursionError,SystemError, or a misreportedion-c error code depending on the code path taken. All paths now raise
IonException, with the original error kept as__cause__. Also bumps thevendored ion-c to v1.1.6.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.