Commit graph

1396 commits

Author SHA1 Message Date
cc9cd5ef28 Rewrite old closet links to use new syntax
A lot of DTI lists use old URLs to anchor-link between lists! Here, we rewrite those URLs to match what DTI 2020 expects, so that they actually correctly jump you across the page and aren't filtered out!
2021-09-06 14:05:36 -07:00
dfcd9985d4 Await closing Playwright before finish request
I noticed when loading Your Outfits earlier (before I switched it to just use prod images even on dev), that there was a big memory leak slowing down my machine.

My hypothesis is that this is because I wasn't _waiting_ for the resources to tear down before finishing the request, so Vercel terminated the request early, and I further hypothesize that terminating a Playwright session partway through isn't guaranteed to clean up the browser.

Not sure about that! Could have just been that we spun up a lot at once, and a bunch of things went into swap! (But I thought it generally handles requests in serial in the dev server? So that feels unlikely.)

Anyway, I don't feel like extensively testing this again and maybe messing up my computer session again :p Just, when I first wrote this without awaits, I knew that it was a bit risky, but I wanted to _see_ if it was a problem before slowing down individual requests by awaiting. And now, my "it's likely to be a problem" threshold has been reached, lol!

So, I'm not _confident_ this is the best play, I don't know the internals well enough; but it seems like a better side to err on than the other, now that I know more!
2021-09-03 15:43:27 -07:00
9a68bd1355 Use standard image URLs on Your Outfits page
The old URLs were glitchy because we weren't escaping the `layerUrls` param… and this will let us take better advantage of the same shared caching as other stuff!
2021-09-03 15:37:38 -07:00
ae6b012f88 Fail harder when *all* layers fail to load
This was actually being _triggered_ by the fact that the user outfits page doesn't generate great outfit image URLs! It doesn't encode the layerUrls parameter, so now that image URLs sometimes contain `&`, the parameter was being misparsed.

Example:
```
http://localhost:3000/api/outfitImage?size=300&layerUrls=https://impress-2020.openneo.net/api/assetImage?libraryUrl=https%3A%2F%2Fimages.neopets.com%2Fcp%2Fitems%2Fdata%2F000%2F000%2F054%2F54348_cf1cfe10c7%2Fall-background.js&size=300,http://images.neopets.com/cp/bio/data/000/000/041/41572_0450defb29/41572.svg,http://images.neopets.com/cp/bio/data/000/000/041/41570_83582a4a83/41570.svg,http://images.neopets.com/cp/bio/data/000/000/041/41571_7e6c072e12/41571.svg,http://images.neopets.com/cp/bio/data/000/000/041/41582_06159c1e4d/41582.svg,http://images.neopets.com/cp/bio/data/000/000/041/41574_520e661a8a/41574.svg,http://images.neopets.com/cp/bio/data/000/000/041/41573_f4f480ba37/41573.svg,https://impress-asset-images.openneo.net/object/000/000/480/480378/300x300.png?v2-1597211608000
```

would get the following list of layer URLs:
```
["https://impress-2020.openneo.net/api/assetImage?libraryUrl=https://images.neopets.com/cp/items/data/000/000/054/54348_cf1cfe10c7/all-background.js"]
```

Anyway, I'm gonna fix that (probably by just not using this layerUrls param anymore and moving to the new outfit ID + timestamp URLs), but let's also just have clearer error messages instead of just a blank image! That way, if something similar happens again, the client will fall back to alt text, instead of showing a blank image.
2021-09-03 14:55:50 -07:00
42628dffab Merge branch 'main' of https://github.com/matchu/impress-2020 into main 2021-09-03 13:45:04 -07:00
443cf3fc26 Close extra browser instances too
I'm seeing a lot of 500 "Could not load image: undefined" lately, not sure why! Wondering if it's resources staying open too long, idk. This should at least be better than leaving it open!
2021-09-03 13:44:19 -07:00
7e0b3b39e7 Oops, right, 150 is a size too 2021-09-03 13:38:37 -07:00
8f845e7264 Better handle null imageUrl in /api/outfitImage
Oops, right, this is a nullable field! Before this change, a null here would crash the function. Now, it returns a helpful error message!
2021-09-03 13:23:19 -07:00
b8081d98aa Add new layer URLs to valid URL patterns 2021-09-03 13:16:36 -07:00
e5bdff7e23
Merge pull request #4 from matchu/dependabot/npm_and_yarn/immer-9.0.6
Bump immer from 8.0.1 to 9.0.6

I tested the preview deploy a little bit with closet manipulation, seemed fine to me! That's where we do our more complex immer stuff anyway, with sets. Most of what we do with immer is very basic!
2021-09-02 19:30:49 -07:00
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
781034a568 Oops, don't crash if animation image fails
Whoops, `Promise.race` isn't quite what I wanted here. This meant that, if the image promise _fails_ before the movie _succeeds_, the outfit would crash even though it doesn't need to. (And this was happening too often, due to a bug in /api/assetImage!)

Now, we accept whichever _successful_ result loads first, or reject if they _both_ fail.

I tested this by having /api/assetImage always throw, and confirmed that it crashed the outfit before this change, and no longer does after this change!
2021-09-02 19:09:27 -07:00
dependabot[bot]
f5b159b808
Bump immer from 8.0.1 to 9.0.6
Bumps [immer](https://github.com/immerjs/immer) from 8.0.1 to 9.0.6.
- [Release notes](https://github.com/immerjs/immer/releases)
- [Commits](https://github.com/immerjs/immer/compare/v8.0.1...v9.0.6)

---
updated-dependencies:
- dependency-name: immer
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
2021-09-03 01:34:24 +00: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
7719ab8c07 Use /api/assetImage for imageUrl
In this change, we start using our new API endpoint for movie image URLs, instead of the Classic DTI image.

This should make the little fade-in phase for certain movies a little bit less jarring (the part where we preload the image before the movie loads), though I suppose that won't necessarily load as fast until it gets into the cache the first time lol. (A good reason to maybe put a more long-lived cache like Fastly in front of this stuff long-term?)

Not doing it for the smaller image sizes yet, I'm a bit worried that I don't 100% know how to teach /api/assetImage to resize without tipping over the function limit…

…oh! I should have the webpage render at different sizes! Yeah that's a great idea lol
2021-08-19 17:40:16 -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
f9b4852da1 Handle OFFICIAL_SWF_IS_INCORRECT for pet layers
Marking this glitch on the Yellow Lutari head today, and oops there isn't UI copy for it yet! Added!

Also fixed some bugs in here, like old text about the position of the pose picker relative to the glitch badge, and I noticed while debugging that `layerUsesHTML5` returns a truthy string instead of a boolean which seems error-prone!
2021-08-14 16:46:05 -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
853296cde2 Empty commit to trigger a deploy
I tried a "Redeploy" from the Vercel console, but it didn't seem to fix
the bug. That could mean that my theory about Node version as the
culprit is wrong? Or it could mean that we need fresh code for that
config change to take effect. So, this commit should simulate fresh
code! lol 😅
2021-08-07 23:40:10 -07:00
0cf97ef612 Try updating honeycomb-beeline?
In production we're suddenly getting errors in module wrapping in honeycomb-beeline. I wonder if it's like, an incompatibility with Vercel's version of Node?

Well, this new version seems to still be playing nice on dev, so hopefully that's all it is and this fixes it! I give it like a 35% chance lol :p
2021-08-07 23:13:52 -07:00
30e6c46d48 Merge branch 'main' of github.com:matchu/impress-2020 into main 2021-08-07 21:44:37 -07:00
e2e7835214 Waka deprecation UI, to expire on Aug 21 2021-08-07 21:32:22 -07:00
fb12f817e2 Add negative word filters to search
Just a quick lil change on a whim, I hope it works well!
2021-07-29 00:25:35 -07:00
2ba18ace4d Add debug lines to appearanceOn Apollo resolver
Hmm, the item page in prod is slower than it is in dev? In dev, most items are satisfied by the preloading in ItemPagePreview, but in prod, those same items need to send a separate OutfitItemsAppearance query _way_ after (which, I think just due to queueing, waits for all the items to wait too).

There's an obvious issue in the case of all the Maraquan items lately, because we just don't do the clever cache lookups for non-standard colors at all. But I don't understand why even standard items like the 17th Birthday Party Hat are struggling!

These are just some simple debug statements, hopefully they'll tell us something about the basics of what's happening!
2021-07-21 16:03:24 -07:00
b7b4fa21ec Oops, show models needed for special colors
Right, oops, `speciesThatNeedModels` is for standard colors! Add in the special colors, too!
2021-07-17 05:47:32 -07:00
f1d24d2177 Clarify the standard colors fit info
I didn't want to use the word "basic", since "basic colors" generally means like Blue, Red, Green, Yellow… but it was the only one that fit in the space lol

I tried a lot of stuff with "Fits standard pets" and stuff and couldn't get it to work well
2021-07-12 04:46:07 -07:00
9e1d8024c1 Lmao whoops I shipped the test data 🤣 2021-07-11 21:32:14 -07:00
29ca2306af Oops, fix items drawn for multiple colors
Just a little display bug on the homepage. For an item like the "Evil Coconut Half Mask", which was specifically drawn for the standard _and_ major special colors, our previous logic would have said "Baby only" or "Maraquan only" or whatever special color it happened to find first.

Now, we only show the case "Baby only" if it _doesn't_ fit standard pets too.

Note that the Maraquan case is tricky, because the Blue Mynci can also wear Maraquan items lol! For this reason, we check for two standard bodies before declaring that it's meant for standard pets.
2021-07-11 21:27:24 -07:00
a1f3c1df13 Oops, fix bug in needed models count!
Lol to test text sizing, I added a "1" to the end of the actual number of models needed 🤣 removed it now!
2021-07-11 19:10:14 -07:00
af493f6719 Add fit info to homepage
I wasn't sure how to fill the space for items that are fully modeled, then realized some basic at-a-glance "who does this fit" would help!

The load time isn't great, I think I need to break out that dependent subquery, but maybe the stale-while-revalidate will cover it well enough at first.
2021-07-11 19:08:47 -07:00
2a5ecb688a Improve homepage modeling loading state
Add a skeleton stripe for the modeling data! Won't show up in most cases because we load fast, but it helps things a lot when it does. (Also, will we keep loading fast with the cache changes on this query?)
2021-07-11 18:16:57 -07:00
f2259d6487 Add modeling info to homepage
To make this fast, I had to tweak the GraphQL resolver a bit to run a filtered version of the query for `newestItems` instead of scanning the full database! But yeah, looking good!

I think I'm gonna want to swap out "Fully modeled" for some insight about who it fits
2021-07-11 18:09:29 -07:00
a19c2facbf Detect more animated items
Okay cool, I noticed that "A Warm Winters Night Background" sometimes animates when other things are playing, but the animations aren't _detected_. (Huh, I actually thought we just didn't schedule ticks in that case? But maybe I'm missing something.)

Anyway, some movies don't use the built-in frames construct to animate, and instead use tweens that hook into the timeline and mutate the stage. Okay! Now we detect those.

This _did_ enable the Play/Pause button on some items that don't actually animate in practice, like the "#1 Fan Room Background", which seems to have an animated string of lights in the corner that got layered incorrectly. Maybe we should add a new glitch type, to flag movies that don't actually animate?
2021-07-02 15:40:04 -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
4e31a4bec7 /internal/assetImage can render movies
This is a new page, that I'm gonna use a headless browser to navigate to and screenshot the asset!
2021-07-02 14:36:08 -07:00
2f0d145e49 Move error toast out of OutfitMovieLayer
Doing this for two reasons! One is that I want the movie layer component to be a bit thinner in general - I think we might even want to move the fallback image logic out, too.

The second is that I want the onError for something else soon!
2021-07-02 14:35:27 -07:00
c8d15fa812 Oops, fix bug in pet appearance support!
Right, oops, this logic was saying "Restricts: None" whenever the appearance restricted _zero or one_ zones, lol 😅
2021-06-29 00:10:42 -07:00
b0672d589f Oops, stop showing PNG reference art
Huh, weird. So I reversed the manifest, because you want to get the *last* movie. And I figured that semantic probably extended to PNGs and SVGs too?

But actually, PNGs sometimes have *other* PNGs in the manifest that aren't the relevant asset at all, and are just reference art.

Again, I'm really not sure what the underlying semantic is here? Does the Neopets customizer just display them all, and for the items with this problem, they happen to layer in a way that's not broken?? I would really like to not do that, and I would really like to know the real semantic, but I can't find it >.>

So um, I'm going ahead and using the best semantic that solves the problems I know about? Which is, use the last movie, and use the first PNG. Fingers crossed lol!

I also didn't test this change extensively, because I'm on a train lol

I'm just trusting that this push will be better than what we had before. I tested it on the Dandan MME, which has two JS files, and it took the latter; and the Pathway of Petals Background, which has two PNG files, and it took the former. Success? 😬🤞
2021-06-27 15:34:36 -07:00
f5e5f16f87 Improve movie cancels and error handling
Oops, my inbox was getting full of uncaught promise rejections of `loadImage`!

I'm pretty sure they're caused when multiple images in a movie fail to load (e.g. network problems), but we fail to cancel them. So, the first failure would be caught as a part of `Promise.all` in `loadMovieLibrary`, but then subsequent failures wouldn't be caught by anything, and would propagate up to the console and to Sentry as uncaught errors.

In this change, we make a number of improvements to cancellation. The most relevant change for this bug is that `loadMovieLibrary` will now automatically cancel all resource promises when it throws an error! But this improved robustness also enabled us to finally offer a simple `cancel()` method on movie library promises, which we now available ourselves of at call sites, too.
2021-06-26 12:04:40 -07:00
0bffaec989 Try moving the glitch badges to the top
Experiment! Let's see if them being more prominent like this is helpful or annoying 😅

I think this is clunkier in the HTML5 Green Happy Path, but worth it for bringing attention in the error cases.

But I feel like we might tweak this over time!
2021-06-24 20:05:31 -07:00
35a1096da3 Fix movie bugs with Dandan, Electric Dress, etc
Huh, so it turns out sometimes the manifest will include old broken conversion attempts!

This fixed the "MiniMME18-S2c: Holomorphic Foliage and Dandan Set", the "Electric Dress" on various species (incl. Aisha), and yeah!

What an interesting discovery 😂
2021-06-24 18:54:48 -07:00
ea33741594 Add search footer to layout, behind a feature flag
Yeah it looks cute as a starting point! Definitely a lot to do here tho 😳
2021-06-21 14:48:08 -07:00
59fe02a4cc Prefer two-column layout on md-size screens
This is because I want to try adding a search footer to the two-column layout, like in Classic DTI—and so I want more screens that _can_ support two-column layout to use it.
2021-06-21 14:14:27 -07:00
27ebe90eef Homepage update June 21 2021-06-21 13:55:54 -07:00