search

luckyframework / lucky

A full-featured Crystal web framework that catches bugs for you, runs incredibly fast, and helps you write code that lasts.
https://luckyframework.org
MIT License
2.59k stars 156 forks source link

Pretty logger still logs () when there's no request id #1626

Closed jwoertink closed 2 years ago

jwoertink commented 2 years ago

In Lucky 0.29.0 there's a new request ID that's logged on each request. It's only supposed to show up when you have a request ID set, but it seems this id https://github.com/luckyframework/lucky/blob/c7c4e0f91278b110e3b4efce270c03a2f8f457fe/src/lucky/pretty_log_formatter.cr#L59

is returning an empty string or something... So you'll see something like

GET / ()
  ...
grepsedawk commented 2 years ago

@jwoertink what is the expectation when request_id is blank? Should it show only GET / with no parenthesis?

jwoertink commented 2 years ago

Yup, exactly. That's what it used to show. Then I added the request_id, and assumed it would be nil here. I guess that's not the case, though, I haven't really looked in to it.

But yeah, to be clear, if there's no request_id, then just show GET / like it used to 😄

grepsedawk commented 2 years ago

Are there any existing test cases you're able to point me to so I can make sure this is regression tested?