Commit graph

77 commits

Author SHA1 Message Date
ea8791c69d Create a new browser for each assetImage request
Well, holding onto the browser instance seems to be a source of bugs (my previous fix didn't seem to fix it), and I'm not _sure_ what the perf characteristics are, so let's just try a fresh instance each time!

I don't actually know what a browser "instance" in Playwright really is, I'm not sure it even necessarily creates a new process, I just don't know

and I saw some Vercel example code take this approach, which is definitely simpler, and I guess must not be _overtly_ bad perf if it's idiomatic?

So, like, ok, cool, let's see if this stops 500ing us with "Browser closed"! 😅
2021-09-02 19:25:48 -07:00
fc52439387 Reboot assetImage browser when it disappears
Been seeing this in testing in prod, the first few images worked great, but then eventually they all started saying the browser was disconnected.

Here, we add a check to reconnect if it goes missing. This is actually kinda hard to test in dev, because the dev server creates a new process every time the function runs, so fingers crossed!

I also added explicit logic to close the page when we're done with it, I'm worried we crashed the browser by exceeding the RAM limit by leaving pages open. Not sure quite how their model works and whether things eventually get flushed out on their own!
2021-08-19 23:38:25 -07:00
f036890aa1 Use /api/assetImage for all image sizes
We update /api/assetImage to accept size as a parameter (I make it mandatory to push people into HTTP caching happy paths), and we update the GraphQL thing to use it in those cases too!

This also means that, if these images seem to go well, we could swap Classic DTI over to them… I want to turn off those RAM-heavy image converters on the VPS lol
2021-08-19 17:56:09 -07:00
adf70dc25f Use chrome-aws-lambda for assetImage
Okay cool, this one worked! We use this special Chrome package with AWS Lambda support, and then we use normal Playright in dev, and then we exclude `playwright` from the deployment (even though it got auto-detected by `require("playwright")`) to just barely sneak in under the 50MB limit for this function. Phew!

The preview deploys for this seem to be, actually working? So that's exciting!
2021-08-19 16:27:22 -07:00
49abb92c03 Use headless chrome in assetImage
Oops, I didn't realize this new package had a headless option! Definitely use it lol, makes dev environment way more chill
2021-08-17 01:44:46 -07:00
c28366772d Polyfill Promise.any for Node v14
Oops, Promise.any was added in Node 15, but prod uses Node 14! Polyfill it!
2021-08-17 01:44:46 -07:00
a1243ad08f Merge branch 'main' of https://github.com/matchu/impress-2020 into main 2021-08-17 01:22:08 -07:00
9ec917e4d3 Use playwright-aws-lambda
So, just using normal playwright was crashing with this error: https://github.com/microsoft/playwright/issues/5862

I didn't understand why everyone was using playwright-core until I read the comments more carefully, and saw that it was because folks were using playwright-aws-lambda, because that's where Vercel functions run. (It has some special compat stuff.)

So I'm figuring that maybe the special case in Vercel's builder that fixes this for playwright-core maybe doesn't apply to normal playwright? But that people don't actually run into that issue in practice, because they're all using playwright-core for playwright-aws-lambda instead?

Idk, let's see how it goes! My hope is that this both fixes the immediate crasher about browsers.json being missing, _and_ fixes a problem we were _gonna_ have down the line about normal playwright not working in an AWS Lambda setting.
2021-08-17 01:01:09 -07:00
ba8e4d8aa7 Trickier disabling honeycomb instrumentation
Hm, okay, so the documented way to not instrument anything doesn't actually stop them from patching Module._load. But this undocumented option sure does! So, woo, let's try it! lol
2021-08-08 00:23:57 -07:00
e5081dab7e Disable honeycomb auto instrumentation
Huh, well, I can't figure out what in our production env stopped working with Honeycomb's automatic instrumentation… so, oh well! Let's try disabling it for now and see if it works.

This means our Honeycomb logs will no longer include _super helpful_ visualizations of how HTTP requests and MySQL queries create a request dependency waterfall… but I haven't opened Honeycomb in a while, and this bug is blocking all of prod, so if this fixes the site then I'm okay with that as a stopgap!

Btw the error message was:
```
Unhandled rejection: TypeError: Cannot read property 'id' of undefined    at exports.instrumentLoad (/var/task/node_modules/honeycomb-beeline/lib/instrumentation.js:80:14)    at Function._load (/var/task/node_modules/honeycomb-beeline/lib/instrumentation.js:164:16)    at ModuleWrap.<anonymous> (internal/modules/esm/translators.js:199:29)    at ModuleJob.run (internal/modules/esm/module_job.js:169:25)    at Loader.import (internal/modules/esm/loader.js:177:24)
```

Oh also, this is the first time eslint has looked at scripts/build-cached-data.js I guess, so I fixed some lint errors in there.
2021-08-08 00:14:55 -07:00
dcfcbf9ff7 /api/assetImage can render movies!
Yeah wow huh, that was much less painful than I expected!

I haven't tested this very rigorously, literally just the Floating Negg Faerie Doll: https://images.neopets.com/cp/items/data/000/000/005/5735_a8feda8d08/5735.js
2021-07-02 15:19:11 -07:00
5a18a1d041 Oops, fix outfit page SSR title
Untitled outfits need a fallback, or else they show "null"!
2021-06-15 22:49:29 -07:00
f932498066 Use a redirect for live outfits instead
I'm gonna try and see about getting Fastly to redirect these internally, so we can get all the benefits of CDN-caching the generated image, without forcing the user through another round-trip!
2021-05-27 18:33:14 -07:00
e608844171 Add better 404 message for outfit image 2021-05-27 17:01:20 -07:00
5f0089f990 Access-Control-Allow-Origin: * for GraphQL
Someone asked to use the DTI API for a small client-side project, so I'm making this change to support it!

As explained in the comment, I think this should be safe regarding CSRF attacks. But it _does_ increase the risk that someday we change something elsewhere that creates a problem, like using cookies to authorize something. So, let's remember to be careful! (as I would hope we would be when adding another auth mechanism!)
2021-05-27 16:41:52 -07:00
2543f89255 Add ~live outfit image URLs
Gonna have the /outfit-urls page start returning these instead, for feature parity with before

I might change the strategy on this at some point, like have it get `updatedAt` and redirect instead of generating the image. But this is simpler for now (and the Vercel cache doesn't seem to be as aggressive as I want anyway), and I can change it later!
2021-05-26 19:44:35 -07:00
7f0a450480 Apply sampling rates to Honeycomb events
Oops, we get a _lot_ of outfit image requests, and it's pushing the limits of our free Honeycomb plan! But I don't really need all that much detail, because there's so many.

So, we here apply sampling! `api/outfitImage` is getting a 1/10 rate, and for GraphQL, `ApiOutfitImage` is getting 1/10, and `SearchPanel` is getting 1/5.

I had to add a `addTraceContext` call, to give all the child events awareness of what operation they're being called in, too!

I haven't actually tested that this is working-working, just that the endpoints still return good data. We'll see how it shakes out in prod!

But I did add `console.log(sampleRate, shouldSample, data);` to the `samplerHook` briefly, to see the data flow through, and I reloaded a `SearchPanel` request a few times and observed a plausibly 20% success rate.
2021-05-26 18:50:19 -07:00
a676be0f0f Use impress-outfit-images.openneo.net outfit URLs
Oops, right, I meant to use the new `impress-outfit-images.openneo.net` host for this! It works just fine from `impress-2020.openneo.net` as the backing source right now, but I want these semi-permanent URLs to be a bit more decoupled.
2021-05-20 20:57:06 -07:00
0eec503b91 Create pretty route for outfit images
This is mostly for decoupling's sake: I want pretty outfit URLs that aren't dependent on the `/api/outfitImage` path structure, so that we can serve them as ~permanent pretty URLs that don't actually indicate where or how the image is stored.
2021-05-20 20:35:41 -07:00
ae82a6236f Revert "Use current host for outfit share tags"
This reverts commit a0121f09ea.

I didn't realize that VERCEL_URL wouldn't use the canonical URL. And thinking further about the Open Graph semantics, I think having just one canonical host makes more sense, even if it makes testing a bit more annoying in some cases.
2021-05-14 21:09:09 -07:00
a0121f09ea Use current host for outfit share tags
This will help test preview pages and such!
2021-05-14 20:47:15 -07:00
d31809cc62 Use VERCEL_URL in /api/outfitImage
Ah right, this is how we get the deployed API code to know its own hostname! This way, when we do a preview deploy, /api/outfitImage will use the preview version of the GraphQL endpoint, too.
2021-05-14 20:28:33 -07:00
5e657e7547 Fix prod outfit SSR by just using URLs, not build
Hmm, the built copy of the HTML isn't deployed with the API function on Vercel. Ok! Let's just do the same trick as we did for the dev server, and make an HTTP request.

This is fine enough for perf because we're caching this result locally to the function, plus it should be a fast CDN-cached response anyway.
2021-05-14 20:27:45 -07:00
08347ad88f Add a note explaining our outfit SSR decision 2021-05-14 20:04:46 -07:00
0fafeb92e1 Oops, fix production outfit SSR
That's what I get for not testing lol!
2021-05-14 20:02:32 -07:00
34ada18beb Use 600x600 outfit images in shares, not 300x300
Size upgrade, wowie! (I figure sharing services probably prefer a higher-resolution image, they're not very big, and to then scale them down for the application-specific use case & device.)
2021-05-14 19:55:12 -07:00
f48e6ba03d Add support for 600x600 outfit images
Gonna use this in the sharing, come next commit!
2021-05-14 19:54:18 -07:00
8f83ac412c Add sharing meta tags to outfit pages
Meta tags are a bit tricky in apps built with `create-react-app`! While some bots like Google are able to render the full page when crawling, not all bots are. Most will just see the empty-ish index.html that would normally load up the application.

But we want outfit sharing to work! And be cool! And use our new outfit thumbnails!

In this change, we add a new server-side rendering API route to handle `/outfits/:id`.

It's very weak server-side rendering: it just loads index.html, and makes a few small tweaks inside the `<head>` tag. But it should be enough for sharing to work in clients that support the basics of Open Graph, which I think most major providers respect! (I know Twitter has their own tags, but it also respects the basics of OG, so let's see whether there's anything we end up _wanting_ to tweak or not!)
2021-05-14 19:51:48 -07:00
9722addd3f Generate outfit images by ID alone
Not using this anywhere in-app yet! But might swap it into the user outfits page, and use it to server-side-render social sharing meta tags!

Also eyeing this as a way to replace our nearly 1TB of outfit image S3 storage, and save $20/mo…
2021-05-13 18:03:56 -07:00
2c043adbe0 Support new Fastly S3 domain in outfit images
Oops, I forgot to grep outside `src` for this, lol! I'll keep the S3 domain support for now, because it's still fine to accept and some clients might be on old code, whatever!
2021-05-13 01:15:17 -07:00
8c8e845dc5 Fix eslint for /api
Oops, I didn't notice this when bulk-fixing eslint stuff in src!
2021-05-13 01:13:21 -07:00
e56c5d9a9c Use stale-while-revalidate for /api/validPetPoses
Idk I was surprised to notice that /api/validPetPoses wasn't cached by the Vercel CDN… but I guess that makes sense, because it's only allowed to be an hour old, and client caches hold onto it, so I suppose it makes sense for the CDN cache to not bother.

But in practice, that means users loading the site are pretty _likely_ to see /api/validPetPoses taking its full load time, which can be up to 500ms.

So, I'm offering this additional cache hint, to be willing to serve /api/validPetPose responses up to a week old while reloading the latest data. My hope is that the cache algorithm is much more excited about holding onto that, and that it makes the 500ms delay very _rare_ in practice!
2021-05-13 01:05:34 -07:00
fc6cb2dfdd Fix Honeycomb trace labels for API endpoints
I'm learning that top-level traces should use operation_name as well as name, because name is the low-level thing that every trace gets (including child traces like db queries and net requests)!

Also there was an incorrect label in one of these, and validPetPoses was missing it altogether
2021-04-23 12:31:50 -07:00
45cf9a6f1f Use the new Waka item ID field
This will help us disambiguate items like Butterfly Dress with a non-unique name, and also some where the real item name is glitchy so using it as a key is not wise lol
2021-04-07 20:41:53 -07:00
61e7a38b33 Normalize names for Waka, and log mismatches
By printing out this logging data, of item names we have that Waka doesn't, vs item names Waka has that we don't, I was able to solve a lot of them with new code in `normalizeItemName`!

Here's the mismatches that are left at time of writing:

```
[Item: Y, Waka: N] No Waka value for NC DTI item "goldenlulumedallion" (43034)
[Item: Y, Waka: N] No Waka value for NC DTI item "faelliebirthdaybagsurprise" (51350)
[Item: Y, Waka: N] No Waka value for NC DTI item "neopets11thbirthdaycommemorativemysterycapsule" (53448)
[Item: Y, Waka: N] No Waka value for NC DTI item "dyeworksblue:iscawig-blue" (70896)
[Item: Y, Waka: N] No Waka value for NC DTI item "mysteriousdoorwithlocks" (75601)
[Item: Y, Waka: N] No Waka value for NC DTI item "grapefruitnecklace" (77779)
[Item: Y, Waka: N] No Waka value for NC DTI item "discofeverbackground" (79250)
[Item: Y, Waka: N] No Waka value for NC DTI item "featherflaredshoes" (80047)
[Item: Y, Waka: N] No Waka value for NC DTI item "dyeworksredradioactivemutantmarkings" (80441)
[Item: N, Waka: Y] No NC DTI data for Waka item "7thbirthdaycakeslice#1"
[Item: N, Waka: Y] No NC DTI data for Waka item "7thbirthdaycakeslice#2"
[Item: N, Waka: Y] No NC DTI data for Waka item "7thbirthdaycakeslice#3"
[Item: N, Waka: Y] No NC DTI data for Waka item "8thbirthdayrainbowcupcake"
[Item: N, Waka: Y] No NC DTI data for Waka item "8thbirthdaysparklercupcake"
[Item: N, Waka: Y] No NC DTI data for Waka item "8thbirthdaytiedwithabowcupcake"
[Item: N, Waka: Y] No NC DTI data for Waka item "babyspringdress"
[Item: N, Waka: Y] No NC DTI data for Waka item "butterflydress(fromfaeriefestivalevent)"
[Item: N, Waka: Y] No NC DTI data for Waka item "discofever"
[Item: N, Waka: Y] No NC DTI data for Waka item "dyeworksblue:iscawig"
[Item: N, Waka: Y] No NC DTI data for Waka item "dyeworksred:radioactivemutantmarkings"
[Item: N, Waka: Y] No NC DTI data for Waka item "featherflairedshoes"
[Item: N, Waka: Y] No NC DTI data for Waka item "festivebooktree"
[Item: N, Waka: Y] No NC DTI data for Waka item "floralblackcardigan"
[Item: N, Waka: Y] No NC DTI data for Waka item "grapefruitneckace"
[Item: N, Waka: Y] No NC DTI data for Waka item "mysteriousdoorwithlocksbackground"
[Item: N, Waka: Y] No NC DTI data for Waka item "valiantchampionwings"
[Item: N, Waka: Y] No NC DTI data for Waka item "waxcrayonwig"
[Item: N, Waka: Y] No NC DTI data for Waka item "youngsophiesdress"
```
2021-04-07 17:25:58 -07:00
3ea6107f4f Fix precedence error in allWakaValues query
Oooooops, this was registering as `(is_nc) OR (is_manually_nc AND locale = "en")`. Returned all locales for auto-detected NC items, not necessary!
2021-04-07 16:00:14 -07:00
b0d5ad76f2 Use value="" in /api/allWakaValues
Oops, I missed something important about the Sheets API here! This was causing us to set `{value: undefined}` for some items, which serialized as `{}`.
2021-04-05 13:49:01 -07:00
cefba8c30c Key Waka data by ID, not name 2021-04-03 14:15:10 -07:00
742a5019d8 Add better error handling to /api/allWakaValues 2021-04-01 22:13:44 -07:00
3bd5be03c7 Add /api/allWakaValues
Just a little experiment to see this working in prod! I don't have permission to actually use this data yet, or a UI for it, just mostly want to see it hooked up to the Sheets API correctly and check the prod cache behavior.
2021-04-01 21:57:57 -07:00
8c5c36eb86 Fix outfit thumbnails for new items
New items have ?v= in the query string, which denied requesting them from the outfit thumbnail API route. Now, we allow any query string.

Also, I started allowing PNGs. I don't think any should actually come through here in practice? But it seems safe and forward-looking.
2021-03-12 04:30:00 -08:00
382aa1430e Support new SVG URLs in outfit thumbnails
Huh, I guess TNT changed the folder name from `object` to `items` for new SVGs? Like, some of the old SVGs still are in `objects`, but…

Hmm, I wonder if I should re-run old manifests at some point? I wonder if maybe like, _everything_ changed, and we're just holding onto old URLs now…
2021-01-21 14:59:56 -08:00
0f61d59917 Add "From" to feedback email body
This is just a convenience to make it easier to see at a glance in my inbox whether I should reply to the email, or whether it just goes nowhere 😅
2021-01-18 05:11:17 -08:00
30e0a149be basic outfits page with pet-only thumbnails 2021-01-04 08:10:35 +00:00
eb22290a64 outfit-images: fix prod-only bugs
We needed an extra build step to get node-canvas working! And `setHeader` is fluent in dev, but not in prod? Go figure!
2021-01-04 06:58:09 +00:00
e0af75d927 outfit-images: render actual PNG and SVG layers! 2021-01-04 05:58:19 +00:00
d10fe5f68f outfit-images: add api endpoint
Just returns the placeholder image right now!
2021-01-04 04:21:34 +00:00
9f692b02b8 fix anonymous feedback sending 2020-10-22 23:35:01 -07:00
456a098df9 replace /api/assetProxy with a CDN proxy
When we decided to start out with /api/assetProxy, we didn't know how much the load would be in practice, so we just went ahead and tried it! Turns out, it was too high, and Vercel shut down our deployment 😅

Now, we've off-loaded this to a Fastly CDN proxy, which should run even faster and more efficiently, without adding pressure to Vercel servers and pushing our usage numbers! And I suspect we're gonna stay comfortably in Fastly's free tier :) but we'll see!

(Though, as always, if Neopets can finally upgrade their own stuff to HTTPS, we'll get to tear down this whole proxy altogether!)
2020-10-19 13:24:13 -07:00
59ae70d417 add cpn links to asset proxy, for old impress 2020-10-12 21:13:35 -07:00