Hi I?ve just spend a bit of time debugging an error arising in `loadNamespace`. The bottom line is that the `vI` object is assigned within an `if` block but expected to exist for all of the remaining code. In some cases where the package library has been corrupted or when it resides on a network drive with bad connection this can lead to error messages complaining about `vI` object not existing. Debugging through the error is difficult, both because `loadNamespace` is called recursively through the dependency graph and the error can arise at any depth. And because the recursive calls are wrapped in `try` so the code breaks some distance from the point where the error occurred. I will suggest mitigating this by adding an `else` clause to the `if` block where `vI` gets assigned that warns about potential corruption of the library and names the package that caused the error. I can open a bug report if you wish, but I would require a bugzilla account for that. Otherwise you?re also welcome to take it from here. With best wishes Thomas Lin Pedersen
>>>>> Thomas Lin Pedersen <thomasp85 at gmail.com> >>>>> on Mon, 22 Jan 2018 14:32:27 +0100 writes:> Hi I?ve just spend a bit of time debugging an error > arising in `loadNamespace`. The bottom line is that the > `vI` object is assigned within an `if` block but expected > to exist for all of the remaining code. In some cases > where the package library has been corrupted or when it > resides on a network drive with bad connection this can > lead to error messages complaining about `vI` object not > existing. Debugging through the error is difficult, both > because `loadNamespace` is called recursively through the > dependency graph and the error can arise at any depth. And > because the recursive calls are wrapped in `try` so the > code breaks some distance from the point where the error > occurred. > I will suggest mitigating this by adding an `else` clause > to the `if` block where `vI` gets assigned that warns > about potential corruption of the library and names the > package that caused the error. Not sure this is desirable... in general even though it may well be desirable in your use case... You will be aware that this an important function that maybe called many times, e.g., notably even at R startup time and so must be very robust [hence the many try* settings] and must use messages/warnings that are suppressable etc etc. On reading the source, I tend to agree with you that it looks odd there is no else clause to that if(), but then there may be subtle good reasons for that we don't see now. > I can open a bug report if you wish, but I would require a > bugzilla account for that. Otherwise you?re also welcome > to take it from here. I'll do that for you in any case. Martin Maechler ETH Zurich > With best wishes Thomas Lin Pedersen
> On 22 Jan 2018, at 16.21, Martin Maechler <maechler at stat.math.ethz.ch> wrote: > >>>>>> Thomas Lin Pedersen <thomasp85 at gmail.com <mailto:thomasp85 at gmail.com>> >>>>>> on Mon, 22 Jan 2018 14:32:27 +0100 writes: > >> Hi I?ve just spend a bit of time debugging an error >> arising in `loadNamespace`. The bottom line is that the >> `vI` object is assigned within an `if` block but expected >> to exist for all of the remaining code. In some cases >> where the package library has been corrupted or when it >> resides on a network drive with bad connection this can >> lead to error messages complaining about `vI` object not >> existing. Debugging through the error is difficult, both >> because `loadNamespace` is called recursively through the >> dependency graph and the error can arise at any depth. And >> because the recursive calls are wrapped in `try` so the >> code breaks some distance from the point where the error >> occurred. > >> I will suggest mitigating this by adding an `else` clause >> to the `if` block where `vI` gets assigned that warns >> about potential corruption of the library and names the >> package that caused the error. > > Not sure this is desirable... in general even though it may well > be desirable in your use case... > > You will be aware that this an important function that maybe > called many times, e.g., notably even at R startup time and so > must be very robust [hence the many try* settings] and must use > messages/warnings that are suppressable etc etc.I absolutely agree that even small changes in execution speed would be bad in such a low-level function. Still, the else clause would only get called in the event an error should be thrown so I can?t envision any performance regression.> > On reading the source, I tend to agree with you that it looks > odd there is no else clause to that if(), but then there may > be subtle good reasons for that we don't see now.If there are reasons for the current construct, then that should of course be taken into account. As far as I can parse every code branch that follows the if statement ends up referencing vI, but lazy evaluation might result in those expression to never be evaluated so it might be valid calls in some circumstances..? Anyway, I can accept the argument that changing it might break things in unexpected ways :-)> >> I can open a bug report if you wish, but I would require a >> bugzilla account for that. Otherwise you?re also welcome >> to take it from here. > > I'll do that for you in any case.thanks best Thomas> > Martin Maechler > ETH Zurich > > >> With best wishes Thomas Lin Pedersen[[alternative HTML version deleted]]