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

Add HTTP TooManyRequests error #719

Merged
merged 1 commit into from
Aug 31, 2022

Conversation

fibonacci1729
Copy link
Contributor

Signed-off-by: Brian H brian.hardock@fermyon.com

Signed-off-by: Brian H <brian.hardock@fermyon.com>
@radu-matei radu-matei added the sdk label Aug 30, 2022
@@ -58,4 +58,5 @@ enum http-error {
invalid-url,
request-error,
runtime-error,
too-many-requests,
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you remember why we had to duplicate the .wits over here?

Copy link
Contributor Author

@fibonacci1729 fibonacci1729 Aug 30, 2022

Choose a reason for hiding this comment

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

Not at the moment, off the cuff it had something to do with the macro crate not being able to access the workspace root by specifying a relative path from the manifest dir (i.e. CARGO_MANIFEST_DIR).

In other words, the code that gets generated would contain something like (which wasn't valid):

include_str!(concat!(env!("CARGO_MANIFEST_DIR"), "/../..wit/spin-http.wit")));

Where CARGO_MANIFEST_DIR points to the macro crate.

@@ -55,4 +55,5 @@ enum http-error {
invalid-url,
request-error,
runtime-error,
too-many-requests,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just a reminder that this can be a breaking change. I don't see us doing explicit matching on these anywhere in our repos tho, so probably fine.

Aside: it would nice if wit-bindgen had a "non-exhaustive" annotation to emit e.g. Rust's #[non_exhaustive]

@fibonacci1729 fibonacci1729 merged commit ee996c1 into fermyon:main Aug 31, 2022
@fibonacci1729 fibonacci1729 deleted the too-many-requests branch August 31, 2022 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants