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

Update account import #250

Merged
merged 6 commits into from
Sep 7, 2023
Merged

Update account import #250

merged 6 commits into from
Sep 7, 2023

Conversation

n13
Copy link
Collaborator

@n13 n13 commented Sep 7, 2023

The history plugin is officially deprecated and many nodes don't support it

This changes our account import to use a different call in the chain API - should be more stable.

A couple of changes

  • more widely supported way to import accounts - does not rely on the deprecated history plugin
  • profile errors are not errors, just means no profile
  • Loggers shall not throw errors that's just weird

Note: The new call can return accounts multiple times, when an account has owner and active keys that are the same, so it needs to de-duplicate the list.

n13 added 5 commits September 7, 2023 20:23
these can happen on any network error and also happen at 404 - not found - that's not an error, just means no profile exists for the account
@n13 n13 changed the title Bugfix/update account import Update account import Sep 7, 2023
@@ -161,8 +161,8 @@ class RemoteConfigService {
},
"eosTestnet": {
"name": "Jungle4 Testnet",
"endpoint": "https://jungle4.dfuse.eosnation.io",
"fastEndpoint": "https://jungle4.dfuse.eosnation.io",
"endpoint": "http://jungle.eosusa.io/",
Copy link
Contributor

Choose a reason for hiding this comment

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

should we be changing these endpoints in firebase and not locally?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was changing them in firebase, but the local app doesn't update for whatever reason.

The mobile phones were updating correctly

@@ -22,12 +22,13 @@ class ProfileService {
final map = Map<String, dynamic>.from(response.data);
return Result.value(ProfileData.fromJson(map, user.network, []));
} else {
LogHelper.e('get profile status error', stacktrace: StackTrace.current);
LogHelper.i('get profile status error');
Copy link
Contributor

Choose a reason for hiding this comment

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

lets pass the server error ${response.statusMessage} to this log

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

@gguijarro-c-chwy gguijarro-c-chwy left a comment

Choose a reason for hiding this comment

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

overall looks good. minor comments

@n13
Copy link
Collaborator Author

n13 commented Sep 7, 2023

overall looks good. minor comments

I tried to catch the 404 but somehow it's impossible .. dio throws an exception on 404, it doesn't return the status 404.

So it always goes into the catch of the try catch on 404.

It's a little unclean since 404 is not an error, really. But will not pack this into this pr

@n13 n13 merged commit 572f9b2 into main Sep 7, 2023
1 of 3 checks passed
@@ -22,12 +22,13 @@ class ProfileService {
final map = Map<String, dynamic>.from(response.data);
return Result.value(ProfileData.fromJson(map, user.network, []));
} else {
LogHelper.e('get profile status error', stacktrace: StackTrace.current);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this doesn't happen - a 404 ends in the catch block of this try/catch because dio throws

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