New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CSV.parse tries to close a file #4298
Comments
Update: Using it from a
Update: Shorter
|
Yes, that's exactly what I meant. The response from MRI Ruby makes sense. The one produced by JRuby does not. |
(Your reproduce steps were perfect - I added the exact wording of the ArgumentError + the stacktrace from MRI - only for comparison.) Can you reproduce the error using the 1.7.26, which is latest stable release? |
Yes, same behaviour as 1.7.25. |
Actually, this is at feature parity with Ruby 1.9.3.
... installing.
|
Hmmmm, interesting. I'm not sure I would call it a feature, but rather something which was not noticed earlier, and was fixed in 2.x. Wouldn't it make sense to backport the fix? Actually, to a reader, it is not obvious with this message that the argument to parse was nil. The first impression is that it must be a bug in Ruby. On the other hand, the exception thrown by Ruby 2.x clearly points out that the argument to close was incorrect. |
This is a pity, as it is impossible for me to upgrade, even though I would love to upgrade (and, since it is rare that a certain condition applies to a single person on the world, I could imagine that other users are affected too). In any case, there are still some weeks until then, and with some luck, a backport is not so difficult? ;-) |
@rovf part of the issue is that there might be people who have 1.9 code which depends on catching NoMethodError. If we change the exception raised it will break their code. Is that unlikely? I guess I don't know but this is one reason we try and stay as close to MRI as we can. |
IMHO: First, it is unlikely that someone specifically catches NoMethodError in this case, because it is not really logical. More likely, RuntimeError or Exception will be caught. Second, even if someone does, the code would be broken once he upgrades to JRuby 2000. Finally, if someone still uses JRuby 1.7 now near the end of its lifetime, and upgrades a last time, he will for sure read about all the changes, simply because in this state, the list of changes is likely short. So, no, I'm willing to bet a good bottle of German wine, that no code will break just because of this change within the 1.7 line. |
@rovf generally it is our policy to be bug for bug compat with MRI version we are supporting because we cannot know how people will react to those bugs in MRI. I did a search for this and found an example of catching NoMethodError in the first page of results in github: You said this is not logical and it would not be something you would add until you hit the weird error you encountered. Then you might add the most specific error possible in case you later discover another weird error in CSV vs generic rescue which would wallpaper over it. Another point would be that when people do update to JRuby 9k they probably expect some transitioning from 1.9 to 2.x since it is a major update. At least in 2.x we get a sensible error so they should be able to easily correct the problem. |
@enebo: I understand your argument, that you want to be bug-compatible. Just want to notice that it is not backed up by the example you gave. If you look at the code which you found on Github, you see that NoMethodError is caught only for the BAD_HEADERS error. In particular, a nil argument to parse would propagate upwards, because the exception would be re-raised in the 'else' part of the statement. |
@headius: JRuby 9k requires Java 1.7. In many (large) enterprises - for example a really huge company I'm working for -, the requirement for enterprise solution is still Java 1.6. This might sound surprising, but has a simple reason: For security reason, any new software needs to get internal certification. The problem in this respect is not so much for the JVM itself, but from the application server and other applications running on it. If the application server and/or some of the business software happened to be written, when JVM 1.6 is the target platform, the requirement is to stick with 1.6, until everything is certified with 1.7. In big companies, this is a tedious process, and usually not one which is considered "urgent" (because new developments can, technically, be done in Java 1.6 as well). Of course, this state won't keep for ever, but I still expect JVM 1.6 environments to be around for another couple of years. |
@rovf I did find a more ambiguous error (it would catch that error but it wraps more code than just that): https://github.com/slctd/tigr-crm/blob/7009bd4420dbefb3669310420f4adea54c8e9d09/lib/importer.rb#L63 In any case, you do understand why I made the point I did so I will not belabor the point any longer. The longer I support this project the more I think someone is doing something somewhere :) |
I also have dealt with wanting 'newer' std-libraries in JRuby 1.7 ... its usually possible since - its just Ruby.
@rovf also I love "old" enterprise deployments - feel free to get in touch when help is needed :) |
Code to execute:
require 'csv'; CSV.parse nil
Environment
jruby 1.7.25 (1.9.3p551) 2016-04-13 867cb81 on Java HotSpot(TM) 64-Bit Server VM 1.7.0_79-b15 +jit [Windows 7-amd64]
CYGWIN_NT-6.1 CMTCL033974 2.4.1(0.293/5/3) 2016-01-24 11:26 x86_64 Cygwin
Expected Behavior
An exception of type ArgumentError, saying that the argument to parse is incorrect.
Actual Behavior
An exception of type NoMethodError :
irb(main):031:0> CSV.parse nil
NoMethodError: undefined method
close' for nil:NilClass from /C:/cygwin64/home/fisrona/gitwrk/vp5/repository/maven2/org/jruby/jruby-complete/1.7.25/jruby-complete-1.7.25.jar!/META-INF/jruby.home/lib/ruby/1.9/csv.rb:1381:in
parse'from (irb):31:in
evaluate' from org/jruby/RubyKernel.java:1079:in
eval'from org/jruby/RubyKernel.java:1479:in
loop' from org/jruby/RubyKernel.java:1242:in
catch'from org/jruby/RubyKernel.java:1242:in
catch' from C:/cygwin64/home/FISRONA/gitwrk/vp5/bin/jirb_swing_ex:64:in
(root)'irb(main):032:0>
The text was updated successfully, but these errors were encountered: