Ogadai / zwift-mobile-api

MIT License
120 stars 25 forks source link

0.1.11 breaks something (seen in zwift-second-screen) #2

Closed jeroni7100 closed 7 years ago

jeroni7100 commented 7 years ago

It seems like 0.1.11 breaks something which gives more 'Failed to get status' in zwift-second-screen (both 0.0.55 and 0.0.56).

Example 1008 active riders in world added 46976 (1) added 210524 (2) added 24617 (3) Failed to get status for 24617

The problems does not occur if I use 0.1.11

wiedmann commented 7 years ago

I'll take a look at the problem. Are you getting any data at all?

jeroni7100 commented 7 years ago

Yes, for me (46976) and the first rider in the list (210524). In the example it fails for 24617. I think it is related to the data of this specific rider, because another test with several followees just now works just fine when this rider is not on course.

On 29 March 2017 at 15:43, wiedmann notifications@github.com wrote:

I'll take a look at the problem. Are you getting any data at all?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Ogadai/zwift-mobile-api/issues/2#issuecomment-290093824, or mute the thread https://github.com/notifications/unsubscribe-auth/APvZC4WgmyWs-Nc3yREnZSf2Seph-PI0ks5rql__gaJpZM4Ms8_h .

Ogadai commented 7 years ago

We sometimes get either a 404 or else no data for riders who otherwise appear to be currently riding (no idea why, other than possibly they're just observing).

Previously those failures were handled, but perhaps something about the wrapper has caused it to fail in a different way?

Ogadai commented 7 years ago

@wiedmann I don't have time to look at it right now, but probably can tomorrow if you haven't figured it out already.

jeroni7100 commented 7 years ago

Yes, I saw (in a test of ZwiftMap) that reading status for this rider failed consistently AND some other followees on course did not appear either (but no corresponding log message).

On 29 March 2017 at 15:56, Ogadai notifications@github.com wrote:

We sometimes get either a 404 or else no data for riders who otherwise appear to be currently riding (no idea why, other than possibly they're just observing).

Previously those failures were handled, but perhaps something about the wrapper has caused it to fail in a different way?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Ogadai/zwift-mobile-api/issues/2#issuecomment-290097832, or mute the thread https://github.com/notifications/unsubscribe-auth/APvZCzckD2dFxVrg3Qb_35MLds4_7yXgks5rqmMhgaJpZM4Ms8_h .

jeroni7100 commented 7 years ago

(maybe it would be an idea to add a logging feature to zwift-mobile-api for debugging purposes)

On 29 March 2017 at 15:59, Jesper Rosenlund Nielsen < jesper@rosenlundnielsen.net> wrote:

Yes, I saw (in a test of ZwiftMap) that reading status for this rider failed consistently AND some other followees on course did not appear either (but no corresponding log message).

On 29 March 2017 at 15:56, Ogadai notifications@github.com wrote:

We sometimes get either a 404 or else no data for riders who otherwise appear to be currently riding (no idea why, other than possibly they're just observing).

Previously those failures were handled, but perhaps something about the wrapper has caused it to fail in a different way?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Ogadai/zwift-mobile-api/issues/2#issuecomment-290097832, or mute the thread https://github.com/notifications/unsubscribe-auth/APvZCzckD2dFxVrg3Qb_35MLds4_7yXgks5rqmMhgaJpZM4Ms8_h .

wiedmann commented 7 years ago

I'm having trouble reproducing this issue. I also looked at the profile for rider 24617 and didn't see anything unusual. If you can get the whole exception information, it might help.

I'll keep looking - maybe it has something to do with being part of a group ride or something.

Ogadai commented 7 years ago

https://github.com/Ogadai/zwift-second-screen gets all the riders you're following, checks which ones are riding (from the all-riders list), and gets regular status updates for each of them (some of which sometimes fail).

Don't spend too much time struggling with it - I'll check it out in the morning.

wiedmann commented 7 years ago

I fixed one issue like this where a new field I defined as int32 was actually an int64, so it's possible there's an optional field that only comes through under certain circumstances that's defined wrong.

It's not one of the 404s, because those have an error message. I think protobufjs exceptions have a blank message, which is why it just says "Failed to get status for 24617" without any error message.

wiedmann commented 7 years ago

It is a protobuf issue. Wrong wire type 6 at offset 94. I'll try to figure out which field is wrong.

Ogadai commented 7 years ago

I think this is fixed now in zwift-mobile-api@0.1.12

It's hard to know for sure because the problem doesn't seem to happen very often. I've also added an exception handler for any possible protobuf errors, and made sure the promise gets correctly rejected so it can be handled properly.