Many calls use errno to look up error, but should be using GetLastError

[Taywee]

Description

errno is used instead of GetLastError, resulting in proper detection of an error, but often an incorrect error itself is thrown. According to the functions (for instance, ReadEventLog), the error information should be from GetLastError, not errno. I'm not sure if the other functions are supposed to be using errno or not.

Gem Version

0.6.7 on jruby 9.2.0.0

Windows Version

Windows 7

Replication Case

Before clearing the log

[38] pry(main)> Win32::EventLog.new('Media Center').total_records
=> 1
[39] pry(main)> Win32::EventLog.new('Media Center').read_last_event
=> #<struct Struct::EventLogStruct
 record_number=94,
 time_generated=2018-08-16 08:42:07 -0600,
 time_written=2018-08-16 08:42:07 -0600,
 event_id=0,
 event_type="information",
 category=0,
 source="MCUpdate",
 computer="windows7",
 user=nil,
 string_inserts=
  ["8:41:42 AM - Testing internet connection.\r\n8:41:59 AM - Connection established.\r\n8:42:04 AM - Retrieving Directory.\r\n8:42:05 AM - Processing Directory.\r\n8:42:06 AM - Processing of Directory is complete.\r\n8:42:06 AM - The MCEClientMaintenance package is already up to date.\r\n8:42:07 AM - Update successful.\r\n8:42:07 AM - Next update at 8/17/2018 7:15:09 AM.\r\n"],
 description=
  "8:41:42 AM - Testing internet connection.\r\n8:41:59 AM - Connection established.\r\n8:42:04 AM - Retrieving Directory.\r\n8:42:05 AM - Processing Directory.\r\n8:42:06 AM - Processing of Directory is complete.\r\n8:42:06 AM - The MCEClientMaintenance package is already up to date.\r\n8:42:07 AM - Update successful.\r\n8:42:07 AM - Next update at 8/17/2018 7:15:09 AM.\r\n\r\n">

After clearing the log

[40] pry(main)> Win32::EventLog.new('Media Center').total_records
=> 0
[41] pry(main)> Win32::EventLog.new('Media Center').read_last_event
Errno::ENAMETOOLONG: File name too long - ReadEventLog
from C:/jruby-9.2.0.0/lib/ruby/gems/shared/gems/win32-eventlog-0.6.7/lib/win32/eventlog.rb:750:in `read_last_event'
[42] pry(main)>

Which is clearly the wrong error.

[djberg96]

The last time I checked FFI.errno was really just a wrapper around GetLastError. Have things changed?

It's also possible that JRuby and Ruby have differing implementations of FFI.errno on Windows.

[Taywee]

You may be right. Let me see if it runs the same way on native Ruby, and if so, I'll close this and re-open it in the jruby ffi repository.

[Taywee]

@djberg96
Looks like this fails with the same error in MRI, though it does look like you were right about the errno thing. Turns out both MRI and Jruby's FFI map it correctly to GetLastError. I think this is not an issue with the error code retrieval, but with using SystemCallError itself:

irb(main):002:0> Win32::EventLog.new('Media Center').total_records
=> 0
irb(main):003:0> Win32::EventLog.new('Media Center').read_last_event
Traceback (most recent call last):
        3: from C:/Ruby25-x64/bin/irb.cmd:19:in `<main>'
        2: from (irb):3
        1: from C:/Ruby25-x64/lib/ruby/gems/2.5.0/gems/win32-eventlog-0.6.7/lib/win32/eventlog.rb:750:in `read_last_event'
Errno::ENAMETOOLONG (Filename too long - ReadEventLog)

It looks like ENAMETOOLONG is error 38:

irb(main):008:0> Errno::ENAMETOOLONG::Errno
=> 38

Which ties exactly to ERROR_HANDLE_EOF, the expected error. The issue appears to be that SystemCallError ties to Errno errors in system calls, rather than the actual map of System Error Codes.

I'm not sure if there's an easy and correct fix for this without having a massive error module and a huge lookup table of all System Error Codes given that the documentation doesn't even list possible errors that any given function can encounter.

[djberg96]

Turns out both MRI and Jruby's FFI map it correctly to GetLastError.

I actually objected to this behavior originally because it can cause confusion: ffi/ffi#200

[Taywee]

Yeah, that is actually quite confusing. It also makes FFI bindings look instantly wrong unless you know the internals of the FFI library, when FFI should usually be dead simple and as obvious as possible. Even more confusing because SystemCallError doesn't do this same platform-aware switching, and just defaults to true errno values everywhere, making cases like this throw incorrect errors (like here) when they look like they should be correct, because the same semantics work elsewhere.

[djberg96]

I ended up addressing this with the ffi-win32-extensions library, with the FFI.raise_windows_error method. Apparently I never incorporated it into this library, though.