Skip to content

add extra error logging and take from migration channel#89

Merged
dpetran merged 3 commits intomainfrom
fix/migration-logging
Sep 17, 2024
Merged

add extra error logging and take from migration channel#89
dpetran merged 3 commits intomainfrom
fix/migration-logging

Conversation

@dpetran
Copy link
Contributor

@dpetran dpetran commented Sep 5, 2024

Add extra logging for failures and add a take from the migration channel.

@dpetran dpetran requested a review from zonotope September 5, 2024 20:22
error-ch ([e]
(throw e))
(log/info :migrate/sid "start" (util/current-time-iso) "." ledgers force)
(try
Copy link
Contributor

Choose a reason for hiding this comment

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

This try will never catch anything. The return value of async/<!! could be an exception, but won't be a thrown exception.

You'll need to check the return value, and if an exception then throw it for this try to actually catch it (which is what the go-try macro does). But at that point you don't need the try.

If the main purpose of the throw statements below is to exit the loop, I think you simply make util/exception? checks you do below not call recur.

Not sure if there is anything to catch an exception upstream, but if so you'll need to either do what I describe above or use go-try.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for catching that, I was trying to get this up for Jake quickly last week and I changed it from a go-try to a go-loop while doing build debugging. I've put it back to the way it was.

Copy link
Contributor

@bplatz bplatz left a comment

Choose a reason for hiding this comment

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

LGTM!

@dpetran dpetran force-pushed the fix/migration-logging branch from 6a52c9d to f16e79d Compare September 17, 2024 20:39
@dpetran dpetran merged commit df076cf into main Sep 17, 2024
@dpetran dpetran deleted the fix/migration-logging branch September 17, 2024 21:20
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