885883 - Using middleware for error pages break the response structure our app is expecting

Status: RESOLVED FIXED

(Webmaker Graveyard :: Popcorn Maker, defect)

Description

[Matthew Schranz [:mjschranz]]

For example, let's look at the steps taken when loading in data from a url such as https://popcorn.webmaker.org/editor/37199/edit.

What we expect to happen is:
1. We grab the email on session and look to see if the project not only exists but if it belongs to this person. If either happen, we throw either a 500 or 404 depending on if it existed and if they owned this project.
2. We then try to remix that project. If it didn't exist then it will fail again and then fallback to loading our base empty project.

Step two doesn't happen. Previous, we returned a nice JSON object. Now we just return an HTML page in one gigantic string.

Basically, we never hit https://github.com/mozilla/popcorn.webmaker.org/blob/master/public/src/butter.js#L887-L909 and go straight to https://github.com/mozilla/popcorn.webmaker.org/blob/master/public/src/butter.js#L911-L912. Thankfully our code is robust enough that the app doesn't implode :)

Comment 1

[Jon Buckley [:jbuck]]

If we wanted to be smart about this, could we send a "Accept: application/json" header with the XHR request, and then inside our global error handler check the Accept header so we could return the error as a JSON response instead of an HTML page.

Comment 2

[Matthew Schranz [:mjschranz]]

Attachment: https://github.com/mozilla/popcorn.webmaker.org/pull/74

I think 1 review is fine here. Whoever can first works for me.

Comment 3

[Matthew Schranz [:mjschranz]]

Did Jon's review comments.

Comment 4

[David Humphrey (:humph)]

Comment on attachment 766661 [details] [review]
https://github.com/mozilla/popcorn.webmaker.org/pull/74

Flag me again once Jon has signed off on this.

Comment 5

[Jon Buckley [:jbuck]]

Comment on attachment 766661 [details] [review]
https://github.com/mozilla/popcorn.webmaker.org/pull/74

r+ with nits, add the res.json bits to the 404 handler in server.js as well.

Comment 6

[Matthew Schranz [:mjschranz]]

Comment on attachment 766661 [details] [review]
https://github.com/mozilla/popcorn.webmaker.org/pull/74

Something changed that caused my original intent to break.

Sorry Jon! Need another quick once over. Or Scott if you have time.

Comment 7

[Scott [:thecount] Downe]

Comment on attachment 766661 [details] [review]
https://github.com/mozilla/popcorn.webmaker.org/pull/74

It is working when I try to edit a project while not logged in, it forces a remix.

If I am logged in, it instead loads a blank page.

Comment 8

[Matthew Schranz [:mjschranz]]

Comment on attachment 766661 [details] [review]
https://github.com/mozilla/popcorn.webmaker.org/pull/74

Fixed up.

Comment 9

[Scott [:thecount] Downe]

Comment on attachment 766661 [details] [review]
https://github.com/mozilla/popcorn.webmaker.org/pull/74

Yup, now working with both remix and edit while not signed in, and while signed in via a different account.

Comment 10

[[github robot]]

Commit pushed to master at https://github.com/mozilla/popcorn.webmaker.org

https://github.com/mozilla/popcorn.webmaker.org/commit/196f2ace8b1fe9f416a1d4308dd3f1e03d24747b
Fix Bug 885883 - Return JSON objects for API request fails rather than hitting our error page when request in headers