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.

Sign up for GitHub

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

Jump to bottom

fix: render initial frame image from image handler #271

Closed
wants to merge 3 commits into from
Closed

fix: render initial frame image from image handler #271

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5651c91
fix: render initial frame image from image handler
dalechyn Apr 19, 2024
e40a44a
chore: changesets
dalechyn Apr 19, 2024
5ceb46f
nit: tweak
dalechyn Apr 19, 2024
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
5 changes: 5 additions & 0 deletions .changeset/funny-cups-kick.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"frog": patch
---

Dropped `frog:image` meta tag in favor of having the image handler to render the initial frame image itself. Fixed wrangler/edge deployment issues that were caused by requesting for `frog:image` tag at the frame handler from image handler where it is limited by wrangler/edge api.
16 changes: 16 additions & 0 deletions playground/src/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,22 @@ export const app = new Frog({
],
})
})
.frame('/square', (c) => {
return c.res({
image: <Box grow backgroundColor="red" />,
intents: [
<Button.Redirect location="http://github.com/honojs/vite-plugins/tree/main/packages/dev-server">
Redirect
</Button.Redirect>,
<Button.Link href="https://www.example.com">Link</Button.Link>,
<Button.Mint target="eip155:7777777:0x060f3edd18c47f59bd23d063bbeb9aa4a8fec6df">
Mint
</Button.Mint>,
<Button.Reset>Reset</Button.Reset>,
],
imageAspectRatio: '1:1',
})
})
.frame('/no-intents', (c) => {
return c.res({
image: <Box grow backgroundColor="red" />,
Expand Down
176 changes: 106 additions & 70 deletions src/frog-base.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import { fromQuery } from './utils/fromQuery.js'
import { getButtonValues } from './utils/getButtonValues.js'
import { getCastActionContext } from './utils/getCastActionContext.js'
import { getFrameContext } from './utils/getFrameContext.js'
import { getFrameMetadata } from './utils/getFrameMetadata.js'
import { getImagePaths } from './utils/getImagePaths.js'
import { getRequestUrl } from './utils/getRequestUrl.js'
import { getRouteParameters } from './utils/getRouteParameters.js'
Expand Down Expand Up @@ -353,30 +352,21 @@ export class FrogBase<
for (const imagePath of imagePaths) {
this.hono.get(imagePath, async (c) => {
const url = getRequestUrl(c.req)

const query = c.req.query()
if (!query.image) {
// If the query is doesn't have an image, it is an initial request to a frame.
// Therefore we need to get the link to fetch the original image and jump once again in this method to resolve the options,
// but now with query params set.
const metadata = await getFrameMetadata(url.href.slice(0, -6)) // Stripping `/image` (6 characters) from the end of the url.
const frogImage = metadata.find(
({ property }) => property === 'frog:image',
)
if (!frogImage)
throw new Error(
'Unexpected error: frog:image meta tag is not present in the frame.',
)
// Redirect to this route but now with search params and return the response
return c.redirect(frogImage.content)
}
const origin = this.origin ?? url.origin
const assetsUrl = origin + parsePath(this.assetsPath)

const defaultImageOptions = await (async () => {
if (typeof this.imageOptions === 'function')
return await this.imageOptions()
return this.imageOptions
})()

const {
headers = this.headers,
image,
imageOptions = defaultImageOptions,
} = fromQuery<any>(c.req.query())

const fonts = await (async () => {
if (this.ui?.vars?.fonts)
return Object.values(this.ui?.vars.fonts).flat()
Expand All @@ -385,12 +375,68 @@ export class FrogBase<
return defaultImageOptions?.fonts
})()

const {
headers = this.headers,
image,
imageOptions = defaultImageOptions,
} = fromQuery<any>(query)
const image_ = JSON.parse(lz.decompressFromEncodedURIComponent(image))
const image_ = image
? JSON.parse(lz.decompressFromEncodedURIComponent(image))
: await (async () => {
// Request body will be empty since a request for the image is always done with
// `GET` http method. Thus, the result of the next invocation is intristically the same
// as getting frame context for an initial frame.
const { context } = getFrameContext<env, string>({
context: await requestBodyToContext(c, {
hub:
this.hub ||
(this.hubApiUrl ? { apiUrl: this.hubApiUrl } : undefined),
secret: this.secret,
verify,
}),
initialState: this._initialState,
origin,
})
const response = await handler(context)
Copy link
Member

Choose a reason for hiding this comment

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

If we go down this route, then this will cause a double render cycle on the initial request (one render cycle on the main endpoint, and one on this /image endpoint). This means it will become non-idempotent (ie. operations in the handler will be invoked twice which could cause db/store related bugs).

I wonder if there is a way to do this without invoking the handler again.

Copy link
Member

@jxom jxom Apr 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I still need to play around with the initial image caching issue, but why would WC cache the initial image if cache-control: max-age=0 on the frame endpoint? Shouldn’t it always serve fresh frame data and seems like a WC issue?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The issue is that WC caches an image by url, and if an image changes its url also changes because the compressed image query parameter is changing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Therefore since every "refreshed" image has a unique link, wc will cache the initial one and would expect the initial one to be updated.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If we go down this route, then this will cause a double render cycle on the initial request (one render cycle on the main endpoint, and one on this /image endpoint). This means it will become non-idempotent (ie. operations in the handler will be invoked twice which could cause db/store related bugs).

I wonder if there is a way to do this without invoking the handler again.

You're absolutely right.
We need to think of a different solution then.

Copy link

@madroneropaulo madroneropaulo Apr 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

https://warpcast.com/horsefacts.eth/0x66aac3d2 IMO the cleaner way to handle this. It's a small step for the user but the code is even cleaner. So we could just revert and include instructions in the documentation of provide some sort of middleware to make it easier to use.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@jxom, if idempotency is our main issue in this PR, we could enforce frame image handler to always be invoked after the frame handler.

Currently, frog implements those to be independent from each other - meaning that given a frame response you're given a specific frame image URL that always renders "independently" from a frame. As in fact if you serialize the query parameters correctly, you could render any image even outside of the frame context.

However, all the app clients are fetching frames first, and retrieve image urls as a result, thus the frame image in fact depends on the frame response itself.

I propose to implement an initial frame image storage inside of frog which would enforce triggering only one of the handlers and returning a stores value in another one.

Here's the first example, where a Frame is casted for the first time (meaning that frame is not cached in the app client):

  1. App client requests a frame. Frame handler is executed, where image is one of the returns. If the context.status === "initial", we put the image in a local storage where the key is some kind of unique requestId (can we get it from somewhere?), which stays the same within a single full frame fetch operation..
  2. App client requests for frame image. Image handler is executed. Since frame handler was executed before, we have the image in local storage by the given requestId, so we just return it.

And here's the second example where the app client has the frame cached.
The example is also only applicable for requests where context.status === "initial"

  1. Since the frame response is cached indefinitely, app client makes a fetch for frame image (given that the age of the cached frame image request has surpassed max-age).
  2. Local storage doesn't have a cached value for that specific requestId (because frame handler was not invoked), so the image handler invokes frame handler just to retrieve the image, and doesn't store anything in the local storage since there will be no requests afterwards.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@jxom, if idempotency is our main issue in this PR, we could enforce frame image handler to always be invoked after the frame handler.

Currently, frog implements those to be independent from each other - meaning that given a frame response you're given a specific frame image URL that always renders "independently" from a frame. As in fact if you serialize the query parameters correctly, you could render any image even outside of the frame context.

However, all the app clients are fetching frames first, and retrieve image urls as a result, thus the frame image in fact depends on the frame response itself.

I propose to implement an initial frame image storage inside of frog which would enforce triggering only one of the handlers and returning a stores value in another one.

Here's the first example, where a Frame is casted for the first time (meaning that frame is not cached in the app client):

  1. App client requests a frame. Frame handler is executed, where image is one of the returns. If the context.status === "initial", we put the image in a local storage where the key is some kind of unique requestId (can we get it from somewhere?), which stays the same within a single full frame fetch operation..
  2. App client requests for frame image. Image handler is executed. Since frame handler was executed before, we have the image in local storage by the given requestId, so we just return it.

And here's the second example where the app client has the frame cached. The example is also only applicable for requests where context.status === "initial"

  1. Since the frame response is cached indefinitely, app client makes a fetch for frame image (given that the age of the cached frame image request has surpassed max-age).
  2. Local storage doesn't have a cached value for that specific requestId (because frame handler was not invoked), so the image handler invokes frame handler just to retrieve the image, and doesn't store anything in the local storage since there will be no requests afterwards.

just figured out that all of that is applicable without this requestId, there's no need to preserve the "origin" of the request as the context is empty – GET requests have no payload in them.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

such a "storage" most likely won't be reliable in serverless environments where a worker would kill itself in between frame and frame image requests causing idempotency breakage

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

such a "storage" most likely won't be reliable in serverless environments where a worker would kill itself in between frame and frame image requests causing idempotency breakage

however, the delay between such request is so small that it's less likely that a pod would go to sleep


// Initial frames should never return non-'success' response as there is no `frameData`
// to return an error response against.
if (response.status !== 'success')
throw new Error(
'Unexpected error: Initial frame cannot return an error response.',
)

const image = response.data.image

// `image` should never be of type `string` in this case as `fc:frame:image` is set directly
// to the image URL or it's base64 data representation in case it is string, thus resolving such
// would never trigger this endpoint.
if (typeof image === 'string')
throw new Error(
'Unexpected error: Expected `image` of type `JSX.Element` for the initial frame image.',
)

return parseImage(
<div
style={{
display: 'flex',
flexDirection: 'column',
height: '100%',
width: '100%',
}}
>
{await image}
</div>,
{
assetsUrl,
ui: {
...this.ui,
vars: {
...this.ui?.vars,
frame: {
height: imageOptions?.height!,
width: imageOptions?.width!,
},
},
},
},
)
})()
return new ImageResponse(image_, {
width: 1200,
height: 630,
Expand Down Expand Up @@ -502,35 +548,38 @@ export class FrogBase<

const imageUrl = await (async () => {
if (typeof image !== 'string') {
const compressedImage = lz.compressToEncodedURIComponent(
JSON.stringify(
await parseImage(
<div
style={{
display: 'flex',
flexDirection: 'column',
height: '100%',
width: '100%',
}}
>
{await image}
</div>,
{
assetsUrl,
ui: {
...this.ui,
vars: {
...this.ui?.vars,
frame: {
height: imageOptions?.height!,
width: imageOptions?.width!,
const compressedImage =
context.status === 'initial'
? null
: lz.compressToEncodedURIComponent(
JSON.stringify(
await parseImage(
<div
style={{
display: 'flex',
flexDirection: 'column',
height: '100%',
width: '100%',
}}
>
{await image}
</div>,
{
assetsUrl,
ui: {
...this.ui,
vars: {
...this.ui?.vars,
frame: {
height: imageOptions?.height!,
width: imageOptions?.width!,
},
},
},
},
},
},
},
),
),
)
),
),
)
const imageParams = toSearchParams({
image: compressedImage,
imageOptions: imageOptions
Expand All @@ -542,7 +591,9 @@ export class FrogBase<
: undefined,
headers,
})
return `${parsePath(context.url)}/image?${imageParams}`
return `${parsePath(context.url)}/image${
imageParams.size !== 0 ? `?${imageParams}` : ''
}`
}
if (image.startsWith('http') || image.startsWith('data')) return image
return `${assetsUrl + parsePath(image)}`
Expand Down Expand Up @@ -672,22 +723,8 @@ export class FrogBase<
property="fc:frame:image:aspect_ratio"
content={imageAspectRatio}
/>
<meta
property="fc:frame:image"
content={
context.status === 'initial'
? `${context.url}/image`
: imageUrl
}
/>
<meta
property="og:image"
content={
ogImageUrl ?? context.status === 'initial'
? `${context.url}/image`
: imageUrl
}
/>
<meta property="fc:frame:image" content={imageUrl} />
<meta property="og:image" content={ogImageUrl ?? imageUrl} />
<meta property="og:title" content={title} />
<meta
property="fc:frame:post_url"
Expand Down Expand Up @@ -721,7 +758,6 @@ export class FrogBase<
})}
/>
)}
<meta property="frog:image" content={imageUrl} />
</head>
<body />
</html>
Expand Down
1 change: 0 additions & 1 deletion src/next/getFrameMetadata.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
const metadata = await getFrameMetadata('https://frame.frog.fm/api')
// biome-ignore lint/performance/noDelete: <explanation>
delete metadata['frog:version']
expect(metadata).toMatchInlineSnapshot(`

Check failure on line 8 in src/next/getFrameMetadata.test.ts

View workflow job for this annotation

GitHub Actions / Verify / Test

src/next/getFrameMetadata.test.ts > default 

Error: Snapshot `default 1` mismatched

- Expected
+ Received

@@ -10,8 +10,9 @@
    "fc:frame:button:3:action": "link",
    "fc:frame:button:3:target": "https://github.com/wevm/frog",
    "fc:frame:image": "https://frame.frog.fm/api/image",
    "fc:frame:image:aspect_ratio": "1.91:1",
    "fc:frame:post_url": "https://frame.frog.fm/api?initialPath=%252Fapi&amp;previousButtonValues=%2523A_%252C_l%252C_l",
+   "frog:image": "https://frame.frog.fm/og.png",
    "og:image": "https://frame.frog.fm/api/image",
    "og:title": "Frog Frame",
  }

 ❯ src/next/getFrameMetadata.test.ts:8:20

Check failure on line 8 in src/next/getFrameMetadata.test.ts

View workflow job for this annotation

GitHub Actions / Verify / Test

src/next/getFrameMetadata.test.ts > default 

Error: Snapshot `default 1` mismatched

- Expected
+ Received

@@ -10,8 +10,9 @@
    "fc:frame:button:3:action": "link",
    "fc:frame:button:3:target": "https://github.com/wevm/frog",
    "fc:frame:image": "https://frame.frog.fm/api/image",
    "fc:frame:image:aspect_ratio": "1.91:1",
    "fc:frame:post_url": "https://frame.frog.fm/api?initialPath=%252Fapi&amp;previousButtonValues=%2523A_%252C_l%252C_l",
+   "frog:image": "https://frame.frog.fm/og.png",
    "og:image": "https://frame.frog.fm/api/image",
    "og:title": "Frog Frame",
  }

 ❯ src/next/getFrameMetadata.test.ts:8:20
{
"fc:frame": "vNext",
"fc:frame:button:1": "Features →",
Expand All @@ -20,7 +20,6 @@
"fc:frame:image": "https://frame.frog.fm/api/image",
"fc:frame:image:aspect_ratio": "1.91:1",
"fc:frame:post_url": "https://frame.frog.fm/api?initialPath=%252Fapi&amp;previousButtonValues=%2523A_%252C_l%252C_l",
"frog:image": "https://frame.frog.fm/og.png",
"og:image": "https://frame.frog.fm/api/image",
"og:title": "Frog Frame",
}
Expand Down
6 changes: 1 addition & 5 deletions src/utils/getFrameMetadata.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
const metadata = await getFrameMetadata('https://frame.frog.fm/api').then(
(m) => m.filter((m) => m.property !== 'frog:version'),
)
expect(metadata).toMatchInlineSnapshot(`

Check failure on line 8 in src/utils/getFrameMetadata.test.ts

View workflow job for this annotation

GitHub Actions / Verify / Test

src/utils/getFrameMetadata.test.ts > default 

Error: Snapshot `default 1` mismatched

- Expected
+ Received

@@ -56,7 +56,11 @@
      "property": "fc:frame:button:3:action",
    },
    {
      "content": "https://github.com/wevm/frog",
      "property": "fc:frame:button:3:target",
-   }
+   },
+   {
+     "content": "https://frame.frog.fm/og.png",
+     "property": "frog:image",
+   },
  ]

 ❯ src/utils/getFrameMetadata.test.ts:8:20

Check failure on line 8 in src/utils/getFrameMetadata.test.ts

View workflow job for this annotation

GitHub Actions / Verify / Test

src/utils/getFrameMetadata.test.ts > default 

Error: Snapshot `default 1` mismatched

- Expected
+ Received

@@ -56,7 +56,11 @@
      "property": "fc:frame:button:3:action",
    },
    {
      "content": "https://github.com/wevm/frog",
      "property": "fc:frame:button:3:target",
-   }
+   },
+   {
+     "content": "https://frame.frog.fm/og.png",
+     "property": "frog:image",
+   },
  ]

 ❯ src/utils/getFrameMetadata.test.ts:8:20
[
{
"content": "vNext",
Expand Down Expand Up @@ -66,11 +66,7 @@
{
"content": "https://github.com/wevm/frog",
"property": "fc:frame:button:3:target",
},
{
"content": "https://frame.frog.fm/og.png",
"property": "frog:image",
},
}
]
`)
})
1 change: 0 additions & 1 deletion src/utils/getFrameMetadata.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ type FrameMetaTagPropertyName =

type FrogMetaTagPropertyName =
| 'frog:context'
| 'frog:image'
| 'frog:prev_context'
| 'frog:version'

Expand Down
Loading