Skip to content

Win32Error: Add to_s and inspect methods#3331

Merged
ashie merged 1 commit into
fluent:masterfrom
ashie:followup-#3325-2
Apr 14, 2021
Merged

Win32Error: Add to_s and inspect methods#3331
ashie merged 1 commit into
fluent:masterfrom
ashie:followup-#3325-2

Conversation

@ashie

@ashie ashie commented Apr 14, 2021

Copy link
Copy Markdown
Member

Which issue(s) this PR fixes:
none

What this PR does / why we need it:
Additional fix for #3325 and #3329.
Since the previous code doesn't have to_s method, it shows only Fluent::Win32Error, doesn't show the detail.

Docs Changes:
none

Release Note:
Same with #3325

@ashie ashie requested review from cosmo0920 and kenhys April 14, 2021 05:14
kenhys
kenhys approved these changes Apr 14, 2021
View reviewed changes

@kenhys kenhys left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure #to_s and #message should be implemented separately.
Anyway this change LGTM.

@ashie
Win32Error: Add to_s and inspect methods and remove message
9a2be48 
Since the previous code doesn't have to_s method, it shows only
`Fluent::Win32Error`, doesn't show the detail.

Signed-off-by: Takuro Ashie <ashie@clear-code.com>
@ashie ashie force-pushed the followup-#3325-2 branch from d4cdea6 to 9a2be48 Compare April 14, 2021 06:06
@ashie

ashie commented Apr 14, 2021

Copy link
Copy Markdown
Member Author

I'm not sure #to_s and #message should be implemented separately.

Ah sorry, I already removed #message locally but forgot to push it.

cosmo0920
cosmo0920 approved these changes Apr 14, 2021
View reviewed changes

@cosmo0920 cosmo0920 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable for me. 👍

@ashie

ashie commented Apr 14, 2021

Copy link
Copy Markdown
Member Author

Thanks.

BTW I found a weird behavior of td-agent 4.x on Windows by this patch.
Sometimes WindowsFile returns a weird return code on calling CreateFile against a non-existent file.
Sometimes ERROR_FILE_NOT_FOUND (158) or sometimes ERROR_SHARING_VIOLATION (32).
Probably Ruby's internal code resets the error code.
We may have to add a method to get correct error code to win32-api, like Fiddle::win32_last_error or FFI::LastError.winapi_error

@ashie ashie merged commit 72267d8 into fluent:master Apr 14, 2021
@ashie ashie deleted the followup-#3325-2 branch April 14, 2021 06:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kenhys kenhys kenhys approved these changes

@cosmo0920 cosmo0920 cosmo0920 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ashie @kenhys @cosmo0920