server: reformat/refactor around redirects (again)

[jorgeorpinel]

Second attempt at #973.

[jorgeorpinel]

Weird. This works for me locally (yarn dev) but crashes in the (built) review app (https://dvc-landing-server-redi-btalyj.herokuapp.com/)... Investigating ⏳

[jorgeorpinel]

@iAdramelk @shcheklein related: do you guys know why when you yarn build and yarn start this locally, it tries to enforce HTTPS on localhost:3000? Makes it impossible for me to debug the built app locally.

[jorgeorpinel]

Anyway, looking at https://dashboard.heroku.com/apps/dvc-landing-server-redi-btalyj/logs I see the review app error log is showing this a lot:

2020-02-20T07:43:44.228837+00:00 app[web.1]: TypeError: Cannot read property 'amp' of null
2020-02-20T07:43:44.228839+00:00 app[web.1]:     at Server.findPageComponents (/app/node_modules/next/dist/next-server/server/next-server.js:465:19)
...

which unfortunately is not a very informative stack trace... But per vercel/next.js#7303 (comment) this probably has to do with my changes around url.parse... Will try one of the fixes in that report ⏳

[iAdramelk]

@jorgeorpinel

related: do you guys know why when you yarn build and yarn start this locally, it tries to enforce HTTPS on localhost:3000? Makes it impossible for me to debug the built app locally.

You can temporarily disable this redirect by commenting these lines: https://github.com/iterative/dvc.org/blob/master/src/utils/redirects.js#L37-L39

[shcheklein]

@iAdramelk @fabiosantoscode @jorgeorpinel this is a bug . It should not be redirecting to https locally.

[fabiosantoscode]

@jorgeorpinel NODE_ENV is set to production, making dev false, which makes the redirect stuff issue a redirect to HTTPS. I'll send a PR to avoid such a redirect on localhost.

UPDATE: See #1010

[shcheklein]

@fabiosantoscode I would check if the redirects engine itself also matches on https only (thus preventing us from testing at least some redirects on localhost)

[fabiosantoscode]

@shcheklein path redirects are relative, so they go to localhost. Hostname redirects (man.dvc.org, error.dvc.org) take you to the real site, where HTTPS works.

[fabiosantoscode]

I think I found the reason for the crash you mentioned earlier.

[jorgeorpinel]

Thanks Fabio, that was part of it.

Fixed this now in e51bc64 ! It's actually a kinda bug from Next... It expects that you always call url.parse sending true to its 2nd param (parseQueryString)... See vercel/next.js#7303 (comment)

[jorgeorpinel]

In any case... This review app works well now. https://dvc-landing-server-redi-btalyj.herokuapp.com/

Thanks everyone for all the feedback.

[shcheklein]

👍 LGTM

[jorgeorpinel]

Deployed and double sanity checked on prod. All good! 🎉

[fabiosantoscode]

@jorgeorpinel let me know if you had issues testing redirects locally. I tested them with:

curl -I --header 'host: man.dvc.org localhost:3000/something`

And checked the status code and location header for each redirect. This is also possible with HTTP GUIs like postman.

[jorgeorpinel]

Seems to work properly after #1010. Thanks!