Skip to content
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

Improve logs and dont fail if it is a duplicated order #40

Merged
merged 6 commits into from
Aug 16, 2023

Conversation

anxolin
Copy link
Contributor

@anxolin anxolin commented Aug 11, 2023

This PR do a series of improvements to have better visibility on our indexing so we can:

  • be notified in case of an error
  • can debug the errors easier

Dont show as error when Duplicated Orders

When an order is duplicated means its already in the orderbook. Therefore, we shouldn't mark the indexing as an error. This PR shows a warning but continues. The order will be registered and monitored in the same way as if the registration in the order book was successful.

image

Notify errors in slack

This PR will notify any error in slack

Its not an ideal solution, but a patch until we have the Valory implementation in place

image

Show more context in the logs

Now logs show more context, like the ethereum transaction where the logs were found in the first place.

I found this is useful when debugging issues.

I made it so its part of the model, so now is persisted and logged.

Overengineered solution to fight some logging problems with Tenderkly logs

Tenderly logs have a limitation of 4K, if you reach it, they will disable them so you will miss a lot of important information.

This PR overrides the console.log so, in case you try to log something bigger (a big JSON) it will try to break the message into several logs.

The console log is overridden cause i suspected that one internal library was logging something big, and that was breaking Tendrly. This override helped a lot, and now the logs are more clear, however, i still see from time to time we reach the limitation for some reason I don't understand

@anxolin anxolin requested a review from mfw78 August 11, 2023 17:31
@mfw78 mfw78 deleted the branch main August 12, 2023 00:19
@mfw78 mfw78 closed this Aug 12, 2023
@mfw78 mfw78 reopened this Aug 12, 2023
@socket-security
Copy link

New dependencies detected. Learn more about Socket for GitHub ↗︎

Packages Version New capabilities Transitives Size Publisher
@types/node-slack 0.0.31 None +0 5.83 kB types
node-slack 0.0.7 environment +38 2.28 MB xoxco

actions/register.ts Show resolved Hide resolved
actions/register.ts Show resolved Hide resolved
actions/utils.ts Show resolved Hide resolved
actions/utils.ts Show resolved Hide resolved
actions/utils.ts Show resolved Hide resolved
actions/utils.ts Show resolved Hide resolved
actions/watch.ts Show resolved Hide resolved
actions/watch.ts Show resolved Hide resolved
actions/watch.ts Outdated Show resolved Hide resolved
mfw78
mfw78 previously approved these changes Aug 15, 2023
Copy link
Contributor

@mfw78 mfw78 left a comment

Choose a reason for hiding this comment

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

Looks great! Just minor nits on typos 😃

actions/utils.ts Outdated Show resolved Hide resolved
actions/utils.ts Outdated Show resolved Hide resolved
actions/utils.ts Outdated Show resolved Hide resolved
actions/utils.ts Outdated Show resolved Hide resolved
actions/watch.ts Outdated Show resolved Hide resolved
actions/watch.ts Outdated Show resolved Hide resolved
actions/watch.ts Outdated Show resolved Hide resolved
actions/watch.ts Outdated Show resolved Hide resolved
actions/watch.ts Outdated Show resolved Hide resolved
actions/watch.ts Outdated Show resolved Hide resolved
@anxolin anxolin force-pushed the dont-fail-if-duplicated-order branch from 949e145 to aaf2d81 Compare August 16, 2023 10:41
@anxolin anxolin changed the base branch from fail-if-error to main August 16, 2023 10:41
@anxolin anxolin dismissed mfw78’s stale review August 16, 2023 10:41

The base branch was changed.

anxolin and others added 2 commits August 16, 2023 11:42
Co-authored-by: mfw78 <53399572+mfw78@users.noreply.github.com>
Co-authored-by: mfw78 <53399572+mfw78@users.noreply.github.com>
@anxolin anxolin merged commit 54383f9 into main Aug 16, 2023
2 checks passed
@anxolin anxolin deleted the dont-fail-if-duplicated-order branch August 16, 2023 10:44
@anxolin anxolin restored the dont-fail-if-duplicated-order branch August 16, 2023 10:52
@anxolin anxolin deleted the dont-fail-if-duplicated-order branch August 16, 2023 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants