-
Notifications
You must be signed in to change notification settings - Fork 70
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rethink log levels #522
Comments
Also we definitely need #317. |
Could you give examples of log lines that you think shouldn't be printed? |
Here are some examples:
|
That does sound like a problem so it seems like it should either be a warning or error. If that's the expected behavior, is there a way to differentiate between the case where we expect that result and responses like that (that maybe happen later) that would be unexpected and problematic?
Can you look into what's causing that? That's a very unhelpful error, but there should be an error logged if there is actually something going wrong.
There should be a warning logged when a packet is rejected because it would go below the minimum balance but that message could be a lot shorter and clearer. We should get rid of the part mentioning the redis script and the redundant mention of the account ID. |
@dora-gt Can you remember why the |
@georgeroman Sorry I don't remember. I wish I had time to dig into but recently I'm very hectic to dedicate to my current project. I'm sorry. If I'm free this weekend, I might be able to. |
No worries, I’ll look into it by myself and let you know if I manage to
address it.
…On Mon, 20 Jul 2020 at 07:00, Taiga Nakayama ***@***.***> wrote:
@georgeroman <https://github.com/georgeroman> Sorry I don't remember. I
wish I had time to dig into but recently I'm very hectic to dedicate to my
current project. I'm sorry. If I'm free this weekend, I might be able to.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#522 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AHKY5T5AJBBB4JXUKJGIXDTR4O6PPANCNFSM4JP4L5IA>
.
|
@dora-gt I just managed to fix the issue. I was issuing a payment involving three ILP nodes (as in the |
Fixed this in #703. It took a few attempts, but eventually I saw that it was just being used to convert into F06 reject which would be handled by the caller by forwarding the request on to the next handler. This got me to notice that all invalid stream packets, even the ones which decrypt successfully, are handled by forwarding them on to next layer. Created #704 for this. No need to close this issue however as the logging output of interledger-rs would still need some good ideas. At the moment, there are just lot of logs being produced while nodes in multinode tests cannot be identified. |
It seems that some
Error
level logs are output consistently in tests. I'm not sure it deserves to be calledError
s because we are just ignoring the outputs.Error
generally means we have to do something immediately to fix it IMO. It might be annoying if we are operatinginterledger-rs
on the production environment because it would be alerted consistently.So I think we have to rethink and possibly to fix some of log outputs or its logic.
The text was updated successfully, but these errors were encountered: