899082 - Handle/Prevent incomplete Version instances

Status: RESOLVED WORKSFORME

(Marketplace Graveyard :: Developer Pages, defect, P2)

Description

[Andrew Williamson [:eviljeff]]

http://sentry.mktmon.services.phx1.mozilla.com/mkt/marketplacefirefoxcom/group/354/

The status page for check-in-fx throws the above error.

The consumer and developer pages are similarly broken - I can't find the app id anywhere so I can't see if the statuses are anything special.

The consumer page throws:
http://sentry.mktmon.services.phx1.mozilla.com/mkt/marketplacefirefoxcom/group/599/

and the reviewer review page seems to throw:
http://sentry.mktmon.services.phx1.mozilla.com/mkt/marketplacefirefoxcom/group/395/

Comment 1

[krupa raj[:krupa]]

This also is causing 
https://marketplace.firefox.com/app/check-in-fox/  to 500.

Comment 2

[Mathieu Pillard [:mat]]

The latest version of that app is weird:
- It has no features instance attached
- It has no files attached

Both should have been created when that version was saved...

Comment 3

[Szmozsánszky István [:flaki]]

The app package might have been damaged during upload (I can remember it stalling during upload). The package itself might be corrupt (it was created with 7-zip x64 on Windows 8 x64).

The app package could be downloaded from: https://github.com/flaki/checkinfox/raw/master/checkinfox-0-4-51.zip if that might help finding the root cause of the problem.

Comment 4

[Christopher Van Wiemeersch [:cvan]]

http://sentry.mktmon.services.phx1.mozilla.com/mkt/marketplacefirefoxcom/group/599/

Comment 5

[Mathieu Pillard [:mat]]

Consumer pages error (http://sentry.mktmon.services.phx1.mozilla.com/mkt/marketplacefirefoxcom/group/599/) is avoided by the fix for bug 899232

Remains:
http://sentry.mktmon.services.phx1.mozilla.com/mkt/marketplacefirefoxcom/group/395/
http://sentry.mktmon.services.phx1.mozilla.com/mkt/marketplacefirefoxcom/group/354/

This bug is really about the fact that we sometimes end up with Version instances with no File instances or no AppFeatures instances attached. So, when you try to access features or files on those versions, it fails. We do that fairly often in various places (API, developer pages, reviewer pages) so it causes multiple failures.

We should figure out what happens, add safeguards, maybe logging, and probably automatically throw away those Version instances if we can't prevent it completely from happening, since they are almost useless.

Comment 6

[Andrew Williamson [:eviljeff]]

(In reply to Mathieu Pillard [:mat] from comment #5)
> We should figure out what happens, add safeguards, maybe logging, and
> probably automatically throw away those Version instances if we can't
> prevent it completely from happening, since they are almost useless.

Logging would help us understand if these issues are caused by a failure during the upload flow rather than at a later date at least.  Ideally we shouldn't be just throwing the versions away as it could be confusing for developers (especially if it turns out the fault happens later, after an apparently successful upload).  

So the best outcome would be the status of the version should change to incomplete or similar - though in the short term it might be sufficiently to just throw them away on the assumption they were broken uploads and a missing upload is better than a broken app.

Comment 7

[Andrew Williamson [:eviljeff]]

The app (check in fox) has been manually fixed with some sql so can't be used to demonstrate this bug anymore.

Comment 8

[Christopher Van Wiemeersch [:cvan]]

Thanks, Mat, for comment 5. I'll try to take this if I have bandwidth next week.

Comment 9

[Szmozsánszky István [:flaki]]

I did not want to try this again, but it is entirely possible that the app/version ended up in this broken state because I did not click the "continue" button after I uploaded the new application package -- but closed the tab instead.

I realized this just now, as this second time around I noticed that I don't remember clicking the "Continue" button after the upload. If anything, this might be something worthy of investigating.

Comment 10

[Mathieu Pillard [:mat]]

Did you mean the "continue" button that appears when you select a file and the upload starts, or the "Save Changes" button that appears on the next page after the upload is complete, with the form allowing you to add Version notes, notes for reviewers and select required features ?

Comment 11

[Szmozsánszky István [:flaki]]

No, I do actually mean the continue button that appears after the new package is selected for upload, then subsequently it is automatically uploaded and checked. Not finishing the process and closing the browser tab at this point might have caused the above error.

Comment 12

[Andrew Williamson [:eviljeff]]

changing component - all evidence points to the bug occurring during upload.

Comment 13

[Christopher Van Wiemeersch [:cvan]]

Could someone else take this? I'm probably not much help here. Rob/Mat probably have a better idea of what's up.

Comment 14

[Mathieu Pillard [:mat]]

I'll give it a try

Comment 15

[Mathieu Pillard [:mat]]

Didn't have time for it last week, but with bug 913010 fixed maybe the situation will improve. I don't think we have had new cases of that problem in production, either. Downgrading priority for those 2 reasons.

Comment 17

[Mathieu Pillard [:mat]]

This has happened for this app: https://marketplace.firefox.com/reviewers/apps/review/scottcinemas

A few notes about what was going on:
- It was PENDING
- It had 2 PENDING versions, but one was deleted
- The deleted version was the current_version/latest_version, causing the issue

I spent some time on it but couldn't figure out how it got in this state, playing with version deleting/uploading locally I could not reproduce :(

Comment 18

[Mathieu Pillard [:mat]]

Closing this bug. We haven't seen it happen a lot, I can't see anything in our code that could cause it, I assume it happens when we have server issues. bug 1076949 should prevent it if that's the case.