Skip to content
This repository has been archived by the owner on Aug 17, 2020. It is now read-only.

Fix #645; Enhance exception handling and async update timer; #691

Merged
merged 1 commit into from
Aug 14, 2016
Merged

Fix #645; Enhance exception handling and async update timer; #691

merged 1 commit into from
Aug 14, 2016

Conversation

jakubsuchybio
Copy link
Collaborator

  • Fix (Problems) Reset AuthToken after 30mins doesn't work #645 by Logouting, auto-loging in again and completely reinicializing GameMapPage.
  • Add Exception message propagation from App_UnhandledException.
  • Change void to Task at ToggleUpdateTimer and fix all usages to await.
  • Add link to GameMapPageViewModel into GameClient -> needs refactoring.
  • Temporary fix for compilation error at PlayerProfilePage.xaml.cs:13

@ST-Apps Please revise the code before merging.

I tested it few times and it should work without any user interaction after those 30mins.

@jakubsuchybio
Copy link
Collaborator Author

This is how it looks when DoRelogin() hits:

[05:29:51] Updating map
[05:29:51] Found 0 catchable pokemons
[05:29:51] Found 5 nearby pokemons
[05:29:51] Found 0 nearby PokeStops
[05:29:51] Finished updating map objects
The thread 0x50cc has exited with code 0 (0x0).
The thread 0x5104 has exited with code 0 (0x0).
[05:30:11] Updating map
The thread 0x5434 has exited with code 0 (0x0).
The thread 0x4c88 has exited with code 0 (0x0).
The thread 0x57b4 has exited with code 0 (0x0).
The thread 0x5680 has exited with code 0 (0x0).
The thread 0x56e8 has exited with code 0 (0x0).
The thread 0x1890 has exited with code 0 (0x0).
[Relogin] Started.
[Relogin] Token successfuly changed.
[Relogin] Reloading gps and playerdata.
Requesting PlayerUpdateResponse
[05:30:38] Found 0 catchable pokemons
[05:30:38] Found 0 nearby pokemons
[05:30:38] Found 0 nearby PokeStops
[05:30:38] Finished updating map objects
Requesting GetInventoryResponse
Requesting GetPlayerResponse
Requesting GetInventoryResponse
[Relogin] Restarting MapUpdate timer.
[Relogin] Stopping API via ApiHandledException.
Exception thrown: 'PokemonGo_UWP.Utils.ApiHandledException' in PokemonGo-UWP.exe
Exception thrown: 'PokemonGo_UWP.Utils.ApiHandledException' in mscorlib.ni.dll
Exception thrown: 'PokemonGo_UWP.Utils.ApiHandledException' in mscorlib.ni.dll
Exception thrown: 'PokemonGo_UWP.Utils.ApiHandledException' in mscorlib.ni.dll
Exception thrown: 'PokemonGo_UWP.Utils.ApiHandledException' in mscorlib.ni.dll
Exception thrown: 'PokemonGo_UWP.Utils.ApiHandledException' in mscorlib.ni.dll
Exception thrown: 'PokemonGo_UWP.Utils.ApiHandledException' in mscorlib.ni.dll
Exception thrown: 'PokemonGo_UWP.Utils.ApiHandledException' in mscorlib.ni.dll
[Relogin] ApiHandledException from API handled.
[Relogin] Successfuly ended.
The thread 0x516c has exited with code 0 (0x0).
The thread 0x1878 has exited with code 0 (0x0).
The thread 0x3e08 has exited with code 0 (0x0).
The thread 0x4044 has exited with code 0 (0x0).
[05:31:00] Updating map
The thread 0x57cc has exited with code 0 (0x0).
[05:31:12] Found 0 catchable pokemons
[05:31:12] Found 5 nearby pokemons
[05:31:12] Found 0 nearby PokeStops
[05:31:12] Finished updating map objects
The thread 0x4730 has exited with code 0 (0x0).
The thread 0x33e4 has exited with code 0 (0x0).
[05:31:32] Updating map
[05:31:32] Found 0 catchable pokemons
[05:31:32] Found 5 nearby pokemons
[05:31:32] Found 0 nearby PokeStops

@ST-Apps
Copy link
Owner

ST-Apps commented Aug 14, 2016

Few questions before merging:

  • Why did you change ToggleUpdateTimer from void to Task?
  • Why here you're not doing something like `e.GetType() == typeof(ApiHandledException))?
  • Why did you change UpdatePlayerData to be public?
  • The previous PR was fixed so there's no need to comment this line anymore
  • Also, here you can just use HandleException(e.Exception), there's no need to create a new one

@00raq00
Copy link

00raq00 commented Aug 14, 2016

@ST-Apps

  1. Does he want wait for ToggleUpdateTimer? If so it cant be void
  2. I think that "e is ApiHandledException" is better then "e.GetType() == typeof(ApiHandledException))"
  3. I would like to see HandleException(e) ;) , because all exceptions inherit Exception class.

@ST-Apps
Copy link
Owner

ST-Apps commented Aug 14, 2016

@00raq00 ToggleUpdateTimer doesn't do async things, that's why I asked. I agree that using is is better, basically anything is better than using string comparison with exception's name :)

@jakubsuchybio
Copy link
Collaborator Author

Why did you change ToggleUpdateTimer from void to Task?

async void should be avoided, because when exception or cancelation inside of them occurs, then that void just swallow it up (if i remember correctly) also, you were using ToggleUpdateTimer synchronously even if there is awaitable thing inside, which I think made problems with mapupdating concurrency. Regardless this thing was also messing with relogin, so I changed it to awaitable.

Also, here you can just use HandleException(e.Exception), there's no need to create a new one

Well I thought to so too, but no. If I use e.Exception, then the previous information of exception is completely lost. (The type and also the message) I don't know why. But if e.Exception exception is sent, then I get exception of type Exception with message like "Exception of System.Exception" or something like that. Dunno why... When I use creation of new exception with the e.Message I get just the message, which is enough for me to identify that ApiHandledException.

Why here you're not doing something like `e.GetType() == typeof(ApiHandledException))?

Because of the previous problem. Checking type for ApiHandledException is useless. The only thing that works here is checking for Message string "Relogin completed.". Check for type is just for fallback reasons, if something that I didn't tested happens. For example if this exception is handled from GameClient.cs:423 await ExceptionHandler.HandleException(ex);

Why did you change UpdatePlayerData to be public?

Because I needed to use it in GameClient

The previous PR was fixed so there's no need to comment this line anymore

Ok, I can uncomment it

@ST-Apps
Copy link
Owner

ST-Apps commented Aug 14, 2016

Because I needed to use it in GameClient

That method calls 2 things inside GameClient, then updates things that are not required to be updated on relogin (on relogin player's name, level and experience don't change), so you shouldn't be calling it from GameClient. Can you please tell me why are you calling it?

@jakubsuchybio
Copy link
Collaborator Author

I wanted to recreate completely switch case from GameMapPageViewModel.cs:55
case GameMapNavigationModes.AppStart: to be sure, that every request is updated like they are updated when switching from Login page.
I could try using just those methods in GameClient:

await GameClient.InitializeDataUpdate();

and

await GameClient.UpdateProfile();
await GameClient.UpdatePlayerStats(false);

But I wanted to be sure I am recreating Manual Login as close as possible. But I can test it with just those lines from here.

@ST-Apps
Copy link
Owner

ST-Apps commented Aug 14, 2016

What about this (inside the relog method maybe):

await InitializeClient();
BootStrapper.Current.NavigationService.Navigate(typeof(GameMapPage), GameMapNavigationModes.AppStart);

I can't test it now, but I think it can work for what you need.

@jakubsuchybio
Copy link
Collaborator Author

Yeah, that was the first thing I tried, but it doesn't work. Dunno why. Maybe because we are already on that page. idk

@ST-Apps
Copy link
Owner

ST-Apps commented Aug 14, 2016

Ok, we'll fix this later. Update because it's conflicting and then I'll merge.

@jakubsuchybio
Copy link
Collaborator Author

jakubsuchybio commented Aug 14, 2016

Sure. After merge I will test using only GameClient methods.

Fix #645 by Logouting, auto-loging in again and completely reinicializing GameMapPage.
Add Exception message propagation from App_UnhandledException.
Change void to Task at ToggleUpdateTimer and fix all usages to await.
Add link to GameMapPageViewModel into GameClient -> needs refactoring.
Temporary fix for compilation error at PlayerProfilePage.xaml.cs:13
@ST-Apps ST-Apps merged commit 05f1ab4 into ST-Apps:master Aug 14, 2016
@ST-Apps ST-Apps added the bug label Aug 14, 2016
@ST-Apps ST-Apps added this to the PoGo-UWP 1.1.0 milestone Aug 14, 2016
ST-Apps added a commit that referenced this pull request Aug 14, 2016
00raq00 pushed a commit to 00raq00/PoGo-UWP that referenced this pull request Aug 14, 2016
@jakubsuchybio jakubsuchybio deleted the 30minexpire branch August 15, 2016 17:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(Problems) Reset AuthToken after 30mins doesn't work
3 participants