onyxframework
name clash with halite
both defined a status_message:
How is this usually resolved in Crystal?
Did it cause any troubles? Halite's APIError should have it's own status_message method, which would override Exception's.
Anyway, I think that Onyx should not monkey-patch Exception in this manner.
to reproduce:
require "onyx/http"
require "halite"
Onyx::HTTP.get "/" do |env|
env.response << "Hello, Onyx!"
end
Onyx::HTTP.get "/proxy" do |env|
r = Halite.get("http://localhost:5000")
env.response << r.body
end
Onyx::HTTP.listen
when compiling:
Error in test.cr:13: expanding macro
Onyx::HTTP.listen
^~~~~~
in test.cr:13: expanding macro
Onyx::HTTP.listen
^
in macro 'listen' /Users/qinsi/dev/wechat/tracking/lib/onyx/src/onyx/http.cr:67, line 5:
1. handlers = Onyx::HTTP::Singleton.instance.handlers()
2.
3. server = Onyx::HTTP::Server.new(handlers, logger: Onyx.logger)
4. server.bind_tcp("127.0.0.1", 5000, reuse_port: false)
> 5. server.listen
6.
instantiating 'Onyx::HTTP::Server#listen()'
in lib/onyx-http/src/onyx-http/server.cr:71: instantiating 'super()'
super
^~~~~
in /usr/local/Cellar/crystal/0.28.0/src/http/server.cr:415: instantiating 'Array(Socket::Server)#each()'
@sockets.each do |socket|
^~~~
in /usr/local/Cellar/crystal/0.28.0/src/indexable.cr:187: instantiating 'each_index()'
each_index do |i|
^~~~~~~~~~
in /usr/local/Cellar/crystal/0.28.0/src/indexable.cr:187: instantiating 'each_index()'
each_index do |i|
^~~~~~~~~~
in /usr/local/Cellar/crystal/0.28.0/src/http/server.cr:415: instantiating 'Array(Socket::Server)#each()'
@sockets.each do |socket|
^~~~
in /usr/local/Cellar/crystal/0.28.0/src/http/server.cr:428: expanding macro
spawn handle_client(_io)
^
in macro 'spawn' /usr/local/Cellar/crystal/0.28.0/src/concurrent.cr:97, line 11:
1.
2.
3.
4. ->(
5.
6. __arg0 : typeof(_io),
7.
8.
9. ) {
10. spawn(name: nil) do
> 11. handle_client(
12.
13. __arg0,
14.
15.
16. )
17. end
18.
19. }.call(_io)
20.
21.
instantiating 'handle_client(IO+)'
in /usr/local/Cellar/crystal/0.28.0/src/http/server.cr:462: instantiating 'HTTP::Server::RequestProcessor#process(IO+, IO+)'
@processor.process(io, io)
^~~~~~~
in /usr/local/Cellar/crystal/0.28.0/src/http/server/request_processor.cr:16: instantiating 'process(IO+, IO+, IO::FileDescriptor)'
def process(input, output, error = STDERR)
^
in /usr/local/Cellar/crystal/0.28.0/src/http/server/request_processor.cr:39: instantiating '(HTTP::Handler | Proc(HTTP::Server::Context, Nil))#call(HTTP::Server::Context)'
@handler.call(context)
^~~~
in /usr/local/Cellar/crystal/0.28.0/src/http/server/handlers/compress_handler.cr:12: expanding macro
{% if flag?(:without_zlib) %}
^
in macro 'macro_4625847840' /usr/local/Cellar/crystal/0.28.0/src/http/server/handlers/compress_handler.cr:12, line 12:
1.
2. request_headers = context.request.headers
3.
4. if request_headers.includes_word?("Accept-Encoding", "gzip")
5. context.response.headers["Content-Encoding"] = "gzip"
6. context.response.output = Gzip::Writer.new(context.response.output, sync_close: true)
7. elsif request_headers.includes_word?("Accept-Encoding", "deflate")
8. context.response.headers["Content-Encoding"] = "deflate"
9. context.response.output = Flate::Writer.new(context.response.output, sync_close: true)
10. end
11.
> 12. call_next(context)
13.
instantiating 'call_next(HTTP::Server::Context)'
in /usr/local/Cellar/crystal/0.28.0/src/http/server/handler.cr:26: instantiating '(HTTP::Handler | Proc(HTTP::Server::Context, Nil))#call(HTTP::Server::Context)'
next_handler.call(context)
^~~~
in /usr/local/Cellar/crystal/0.28.0/src/http/server/handlers/error_handler.cr:15: instantiating 'call_next(HTTP::Server::Context)'
call_next(context)
^~~~~~~~~
in /usr/local/Cellar/crystal/0.28.0/src/http/server/handler.cr:26: instantiating '(HTTP::Handler | Proc(HTTP::Server::Context, Nil))#call(HTTP::Server::Context)'
next_handler.call(context)
^~~~
in /usr/local/Cellar/crystal/0.28.0/src/http/server/handlers/log_handler.cr:11: instantiating 'Time.class#measure()'
elapsed = Time.measure { call_next(context) }
^~~~~~~
in /usr/local/Cellar/crystal/0.28.0/src/http/server/handlers/log_handler.cr:11: instantiating 'Time.class#measure()'
elapsed = Time.measure { call_next(context) }
^~~~~~~
in /usr/local/Cellar/crystal/0.28.0/src/http/server/handlers/log_handler.cr:11: instantiating 'call_next(HTTP::Server::Context)'
elapsed = Time.measure { call_next(context) }
^~~~~~~~~
in /usr/local/Cellar/crystal/0.28.0/src/http/server/handler.cr:26: instantiating '(HTTP::Handler | Proc(HTTP::Server::Context, Nil))#call(HTTP::Server::Context)'
next_handler.call(context)
^~~~
in /usr/local/Cellar/crystal/0.28.0/src/http/server/handlers/static_file_handler.cr:30: instantiating 'call_next(HTTP::Server::Context)'
call_next(context)
^~~~~~~~~
in /usr/local/Cellar/crystal/0.28.0/src/http/server/handler.cr:26: instantiating '(HTTP::Handler | Proc(HTTP::Server::Context, Nil))#call(HTTP::Server::Context)'
next_handler.call(context)
^~~~
in /usr/local/Cellar/crystal/0.28.0/src/http/server/handlers/websocket_handler.cr:47: instantiating 'call_next(HTTP::Server::Context)'
call_next(context)
^~~~~~~~~
in /usr/local/Cellar/crystal/0.28.0/src/http/server/handler.cr:26: instantiating '(HTTP::Handler | Proc(HTTP::Server::Context, Nil))#call(HTTP::Server::Context)'
next_handler.call(context)
^~~~
in lib/onyx-http/src/onyx-http/middleware/cors.cr:62: instantiating 'call_next(HTTP::Server::Context)'
call_next(context)
^~~~~~~~~
in /usr/local/Cellar/crystal/0.28.0/src/http/server/handler.cr:26: instantiating '(HTTP::Handler | Proc(HTTP::Server::Context, Nil))#call(HTTP::Server::Context)'
next_handler.call(context)
^~~~
in lib/onyx-http/src/onyx-http/middleware/logger.cr:69: instantiating 'call_next(HTTP::Server::Context)'
call_next(context)
^~~~~~~~~
in /usr/local/Cellar/crystal/0.28.0/src/http/server/handler.cr:26: instantiating '(HTTP::Handler | Proc(HTTP::Server::Context, Nil))#call(HTTP::Server::Context)'
next_handler.call(context)
^~~~
in lib/onyx-http/src/onyx-http/middleware/renderer.cr:36: instantiating 'Array(MIME::MediaType)#each()'
accept.each do |a|
^~~~
in /usr/local/Cellar/crystal/0.28.0/src/indexable.cr:187: instantiating 'each_index()'
each_index do |i|
^~~~~~~~~~
in /usr/local/Cellar/crystal/0.28.0/src/indexable.cr:187: instantiating 'each_index()'
each_index do |i|
^~~~~~~~~~
in lib/onyx-http/src/onyx-http/middleware/renderer.cr:36: instantiating 'Array(MIME::MediaType)#each()'
accept.each do |a|
^~~~
in lib/onyx-http/src/onyx-http/middleware/renderer.cr:42: instantiating 'render_json_error(HTTP::Server::Context, Exception+)'
render_json_error(context, error)
^~~~~~~~~~~~~~~~~
in lib/onyx-http/src/onyx-http/middleware/renderer.cr:130: instantiating 'Exception+#status_message()'
name: error.status_message,
^~~~~~~~~~~~~~
in lib/halite/src/halite/error.cr:52: expanding macro
getter status_message
^
in macro 'getter' expanded macro: macro_4459236080:117, line 5:
1.
2.
3.
4. def status_message
> 5. @status_message
6. end
7.
8.
9.
10.
Can't infer the type of instance variable '@status_message' of Halite::Exception::ServerError
The type of a instance variable, if not declared explicitly with
`@status_message : Type`, is inferred from assignments to it across
the whole program.
The assignments must look like this:
1. `@status_message = 1` (or other literals), inferred to the literal's type
2. `@status_message = Type.new`, type is inferred to be Type
3. `@status_message = Type.method`, where `method` has a return type
annotation, type is inferred from it
4. `@status_message = arg`, with 'arg' being a method argument with a
type restriction 'Type', type is inferred to be Type
5. `@status_message = arg`, with 'arg' being a method argument with a
default value, type is inferred using rules 1, 2 and 3 from it
6. `@status_message = uninitialized Type`, type is inferred to be Type
7. `@status_message = LibSome.func`, and `LibSome` is a `lib`, type
is inferred from that fun.
8. `LibSome.func(out @status_message)`, and `LibSome` is a `lib`, type
is inferred from that fun argument.
Other assignments have no effect on its type.
Can't infer the type of instance variable '@status_message' of Halite::Exception::ServerError
It seems the status_message in onyx prevented the type of status_message in halite to be inferred.
Some wild guesses from me:
-
Try requiring
halitebeforeonyx/http -
Monkey patch Halite's error so
@status_messagehas type:module Halite module Exception class APIError < ResponseError getter status_message : String end end endor
module Halite module Exception class APIError < ResponseError @status_message : String end end endYou could also experiment with the order — it should be possible to monkey-patch Halite before actually requiring it.
P.S: You can use <details> tag to hide big chunks of code in your comments :wink:
Changing require order doesn't work. Monkey-patching works, but have to define it as nil-able:
module Halite
module Exception
class APIError < ResponseError
getter status_message : String | Nil
end
end
end
Still, this feels hacky. Should I report this to halite?
P.S. Nice trick about the <details> tag. 😄
@qszhu,
Should I report this to halite?
No, I don't think so. It's Onyx's issue.
It's not that simple.
Onyx rescues all Exceptions. If server is in verbose mode, the exception name is printed into a response, formatted. For example, Errors::MyCustomError is printed as 500 My Custom Error, and IndexOutOfBounds as 500 Index Out Of Bounds.
This is the extension used in Onyx:
class Exception
# The status message for this error. Returns its class name decorated as
# an HTTP status message, for example `"User Not Found"` for
# `MyEndpoint::UserNotFound` error.
def status_message
{{@type.name.split("::").last.underscore.split("_").map(&.capitalize).join(" ")}}
end
end
This way the exception's formatted name is baked into the binary. There are plenty of places where error.status_message is used, and replacing the baked string with runtime manipulation would affect performance.
It seems like a good idea to rename the method to http_status_message, but there is no guarantee of clash absence in the future.
Another option would be wrapping all rescued exceptions into a generic class:
class Onyx::HTTP::Exception(T)
# The status message for this exception. Returns `T` class name decorated as
# an HTTP status message, for example `"User Not Found"` for
# `Onyx::HTTP::Exception(MyEndpoint::UserNotFound)`.
def status_message
{{T.name.split("::").last.underscore.split("_").map(&.capitalize).join(" ")}}
end
end
class HTTP::Server::Response
# A rescued error which is likely to be put into the response output.
property error : Onyx::HTTP::Exception?
def reset
previous_def
@error = nil
end
end
I think a scoped (within Onyx's namespace) exception class is better, so that it doesn't interfere with other classes with a same name.