Skip to content

Refactor Socket::Addrinfo::Error based on os_error #10761

Merged
straight-shoota merged 2 commits intocrystal-lang:masterfrom
straight-shoota:refactor/addrinfo-error
Jun 4, 2021
Merged

Refactor Socket::Addrinfo::Error based on os_error #10761
straight-shoota merged 2 commits intocrystal-lang:masterfrom
straight-shoota:refactor/addrinfo-error

Conversation

@straight-shoota
Copy link
Member

@straight-shoota straight-shoota commented May 29, 2021

Addrinfo::Error used to have an extra ivar error_code to keep track of the error code. This patch refactors the class to use SystemError#os_error and the associated factory methods instead, which is exactly made for this purpose.
Addrinfo::Error already inherits SystemError through Socket::Error, so this doesn't change the type hierarchy.

As a result, Addrinfo::Error#value and the Addrinfo constructors receiving a plain Int32 error code are being deprecated (they keep working as delegates to the respective SystemError methods.

A necessary addition to SystemError (first commit) is the factory method .os_error_message which allows customization of the error message constructed from the os_error value (which was previously hardcoded to os_error.message). This allows incorporation of opts values into the message.

@straight-shoota straight-shoota added kind:feature kind:refactor breaking-change May have breaking effect in some edge cases. Mostly negligible, but still noteworthy. topic:stdlib:runtime topic:stdlib:networking labels May 29, 2021
Copy link
Member

@beta-ziliani beta-ziliani left a comment

Choose a reason for hiding this comment

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

Nice refactor <2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change May have breaking effect in some edge cases. Mostly negligible, but still noteworthy. kind:feature kind:refactor topic:stdlib:networking topic:stdlib:runtime

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants