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

[FE][CORS-PROXY] Image generation should fail if CORS proxy can't fetch images #788

Open
ryscheng opened this issue Jun 2, 2023 · 4 comments · May be fixed by #858
Open

[FE][CORS-PROXY] Image generation should fail if CORS proxy can't fetch images #788

ryscheng opened this issue Jun 2, 2023 · 4 comments · May be fixed by #858
Assignees
@Jipperism

Comments

@ryscheng
Copy link
Member

ryscheng commented Jun 2, 2023

Which area(s) of Hypercerts are affected? (leave empty if unsure)

Frontend Dapp

To Reproduce

When minting, enter an image URL that doesn't work.
The one we tried was returning 429 rate limit errors, but we should try other forms of errors as well.

Describe the Bug

In the case of 429, the client could see the image in the create form, but the image generation would fail because the CORS proxy was the thing that was being rate limited.

Expected Behavior

If the CORS proxy can't fetch the entered image, we should just have minting fail catastrophically with a good error message. It's not great for a user to end up with a Hypercert that they don't expect (e.g. where the image is missing)
@ccerv1 ccerv1 removed the type:bug label Jun 19, 2023
@bitbeckers bitbeckers linked a pull request Jun 20, 2023 that will close this issue
Feat/image loading errors #858
Open
@ryscheng ryscheng self-assigned this Jun 23, 2023
@ryscheng ryscheng removed their assignment Jul 28, 2023
@bitbeckers bitbeckers changed the title Image generation should fail if CORS proxy can't fetch images [FE][CORS-PROXY] Image generation should fail if CORS proxy can't fetch images Oct 4, 2023
@Jipperism
Copy link
Contributor

@ryscheng does your plasmic also crash when clicking the card inside the HypercertForm component? (https://studio.plasmic.app/projects/38ak4QaubqTqAiZYMMbzfn/-/HypercertForm?arena_type=component&arena=6Urt_8kdS12). I'm trying to assign a new ID to the background but can't open it.

If we can assign a new ID to the background image for the card shown on that page, we can do a 'manual' proxy where we fetch it, convert to a b64 string and set it manually in the background image. That way the canvas doesn't become tainted and we should be able to render without using the proxy configuration. Doing the proxy through the built-in configuration will always be prone to rate limit errors etc, because you can't be sure whether the second request will resolve as well, after fetching it manually to determine if the image is available.

@ryscheng
Copy link
Member Author

ryscheng commented Oct 30, 2023 •
edited

It looks like the HypercertForm was removed in the project?
So now I see a bunch of red errors because the FormField cannot get the form props

Feel free to DM me if I can be of help debugging something
Screen Shot 2023-10-30 at 15 24 40

@Jipperism
Copy link
Contributor

I'm not sure what happened there, but looking at it again I think the reason is actually that the HypercertImage component lives in the Hypercerts Core project, which I don't have access to. Maybe @ryscheng or @holkeb can grant me access?

The HypercertForm component does show up for me in plasmic, I just don't think it's wrapped in the correct wrapper hence it doesn't receive those props. Might have something to do with me trying to generalize it for the BlueprintCreateForm.

@ryscheng
Copy link
Member Author

Oh! Thanks for flagging that, it looks like we've been sharing access by the individual project. I just added you and @bitbeckers to the workspace. Sorry about that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Jipperism Jipperism

Labels
None yet
Projects
hypercerts general
Status: Up Next
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Feat/image loading errors hypercerts-org/hypercerts
4 participants
@ryscheng @Jipperism @bitbeckers @ccerv1