Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: handle 404 on cloud functions #105

Merged
merged 16 commits into from
Jun 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions .github/workflows/firebase-qa-pull-request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ jobs:
submodules: recursive
- uses: actions/setup-node@v2
with:
node-version: '12'
node-version: '14'
- name: Cache node modules
uses: actions/cache@v2
env:
Expand Down Expand Up @@ -42,9 +42,23 @@ jobs:
expires: 7d
env:
FIREBASE_CLI_PREVIEWS: hostingchannels
- name: clear cj envs to access status org
uses: w9jds/firebase-action@master
with:
args: functions:config:unset gh.token gh.user
env:
FIREBASE_TOKEN: '${{ secrets.FIREBASE_TOKEN }}'
PROJECT_ID: qa-ccv-brown-edu
- name: set cj envs to access status org
uses: w9jds/firebase-action@master
with:
args: functions:config:set gh.token='${{ secrets.CJ_GH_REPO_TOKEN }}' gh.user='${{ secrets.CJ_GH_USER }}'
env:
FIREBASE_TOKEN: '${{ secrets.FIREBASE_TOKEN }}'
PROJECT_ID: qa-ccv-brown-edu
- name: Deploy to Firebase
run: npm run qa-deploy
env:
FIREBASE_TOKEN: ${{ secrets.FIREBASE_TOKEN }}
FIREBASE_TOKEN: '${{ secrets.FIREBASE_TOKEN }}'
- name: Remove content folder (extra git module instance confuses gh)
run: rm -rf functions/content
65 changes: 54 additions & 11 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,49 @@
# ccv-website-experimental
# CCV's Website

> CCV Website Experimental in Nuxt.js
Developed in Nuxt.js, deployed to firebase.

## Dev Setup
## General Set Up

Our website is partly static and partly dynamic. The static content is mostly handled via [Nuxt Content](https://content.nuxtjs.org) and it lives in the [ccv-website-content repository](https://github.com/brown-ccv/ccv-website-content). Generally, our members will contribute to the content repository. Only structural contributions are handled in this repo. The content repo is included here as a git submodule for development/deployments purposes only. Content should not be modified within here (except for testing purposes).

We use `nuxt generate` in conjunction with Firebase static hosting to serve our static content. During the build process `nuxt generate` places all static html pages into the `dist` folder. If a route is found in the `dist` folder, Firebase serves that, if a route is not found, we rely on cloud functions to handle the reminder of the routes.

The part of [Firebase's configuration](firebase.json) that makes this happen is

```
"rewrites": [
{
"source": "**",
"function": "ssrapp"
}
],
```

In order to handle our 404 page correctly, we control which routes are served dynamically by the Nuxt app. As of (7-22-2022), the only dynamic routes configured in [functions/index.js](functions/index.js) are:

```
let dynamicRoutes = ['/_ghapi/status', '/_workday/opportunities']
```

All other routes that are not included on that list or in the `dist` folder are redirected to a pre-generated `404.html`. By default, Nuxt does not generate a `404.html` (only a 404.vue), to get the html page, we added the following block in the config.

```
generate: {
fallback: "404.html"
},
```

## GitHub Actions

There are two main workflows:

1. When a PR is opened against the main branch, we deploy to qa-ccv-brown-edu live and preview channels.
There is no such thing as "preview" functions, so if the changes include changes to the functions, one needs to check qa-ccv-brown-edu.web.app (main channel). If the changes are only to static pages, then the preview channel will reflect the changes appropriately.
2. When a PR is merged to the main branch, we deploy to ccv-brown-edu (live channel and functions)

![GitHub Actions Flow](actions-flow.png)

## Local Dev Setup

```bash
# install dependencies
Expand All @@ -20,28 +61,30 @@ git submodule update --remote
$ npm run dev
```

Add a .env file with the below keys (token needs read access to ccv-status repo) for the status banner to work (will gracefully degrade in dev mode if not available):
You'll need an .env file with the below keys (token needs full repo access to ccv-status) for the status banner to work (will gracefully degrade in dev mode if not available):

```
GITHUB_USER=
GITHUB_TOKEN=
```

For detailed explanation on how things work, check out [Nuxt.js docs](https://nuxtjs.org).


## Testing build set up

In deployment, the dynamic routes are served by Firebase functions. To simulate this behavior, we need to run through the entire build process, which asks Nuxt to generate the static routes and firebase to emulate the functions

```
npm install -g firebase-tools #if needed
npm run fn-install # if needed
npm run local-deploy
```

May need to set up github credentials with functions env:
```
firebase functions:config:set gh.token="THE API KEY" gh.user="THE CLIENT ID"
```
**Note:**
* 2022-06-23 (MIR): Our local-deploy uses `firebase serve` which seems to not be recommended anymore. We'll probably need to run the functions emulator instead.

## Useful guides

Main guide used :
* [Nuxt.js docs](https://nuxtjs.org).
* https://github.com/benmayer/nuxt-ssr-firebase-template
With it's corresponding post
* https://ben-mayer.com/blog/building-a-web-app-using-nuxtjs-and-firebase
Expand Down
Binary file added actions-flow.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
14 changes: 12 additions & 2 deletions functions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ let isReady = false;

let nuxt = loadNuxt('start');

let dynamicRoutes = ['/_ghapi/status', '/_workday/opportunities']
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd consider renaming this to whitelistedRoutes, that way we know that these routes are allowed by the cloud function. It's the normal language used for this sort of thing in networking. Obviously this doesn't matter either ;-)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t love the whitel/black list terminology, but I could use allowedRoutes or something more clear


async function handleRequest(req, res) {
try {
if (!isReady) {
Expand All @@ -19,11 +21,19 @@ async function handleRequest(req, res) {
}
console.log(req.path);
res.set('Cache-Control', 'public, max-age=3600, s-maxage=7200');
await nuxt.server.app.handle(req, res, (out) => console.log(out));
if(dynamicRoutes.includes(req.path)){
await nuxt.server.app.handle(req, res);
}
else{
Comment on lines +26 to +27
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd stick this on one line, not that it matters though...

console.log('Redirect to 404 page');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need this log still?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not

res.set('X-Cascade', 'PASS')
res.status(404).redirect('/404.html')
Comment on lines +29 to +30
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think y'all might need a style enforcer like prettier or standard I'm seeing ; sometimes, but not others

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably… we have it in the parent dir… may just need to add it to the functions dir..

}

} catch (error) {
console.log(error);
process.exit(1);
}
}

exports.ssrapp = functions.https.onRequest(handleRequest);
exports.ssrapp = functions.https.onRequest(handleRequest);
3 changes: 3 additions & 0 deletions nuxt.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,4 +117,7 @@ export default {
await addSearch(nuxt);
},
},
generate: {
fallback: "404.html"
},
};
Loading