Add email worker and send email support #624
Add email worker and send email support #624LuisDuarte1 wants to merge 2 commits intocloudflare:mainfrom
Conversation
LuisDuarte1
changed the title
LuisDuarte1
force-pushed
the
lduarte/email-support
branch
2 times, most recently
from
15deda7 to
d639b16
Compare
LuisDuarte1
force-pushed
the
lduarte/email-support
branch
2 times, most recently
from
75e3e45 to
a72c76b
Compare
LuisDuarte1
force-pushed
the
lduarte/email-support
branch
from
a72c76b to
c712ffd
Compare
|
|
||
| // For some reason, email events doesn't seem to use WorkerEntrypoint so we get the env and ctx from | ||
| // from the function itself. | ||
| async email(message, _env, _ctx) { |
There was a problem hiding this comment.
@ObsidianMinor do you know why email worker wouldn't use the new interface? Will cron triggers have the same issue?
There was a problem hiding this comment.
I still haven't figured it out yet - for some reason env is still not probably propagated into the rust layer, and therefore I can't seem to use bindings with it 😅
There was a problem hiding this comment.
Its also quite difficult to debug because email workers can't be run locally 😅
|
|
||
| // TODO(lduarte): see if also accepting string is needed | ||
| #[wasm_bindgen(constructor, catch)] | ||
| pub fn new(from: &str, to: &str, raw: &str) -> Result<EmailMessage, JsValue>; |
There was a problem hiding this comment.
Should we also have a new_from_stream method that takes ReadableStream?
| } | ||
|
|
||
| #[wasm_bindgen] | ||
| extern "C" { |
There was a problem hiding this comment.
Should we add headers field? Should these getters also be on EmailMessage?
| pub fn forward( | ||
| this: &ForwardableEmailMessage, | ||
| rcpt_to: &str, | ||
| headers: Headers, |
There was a problem hiding this comment.
This field should probably be optional, or maybe have forward and forward_with_headers. Or is there no difference between headers being undefined or {} in JavaScript API?
| use wasm_bindgen::JsCast; | ||
| use wasm_bindgen::JsValue; | ||
| use wasm_bindgen_futures::JsFuture; | ||
| use worker_sys::EmailMessage as EmailMessageExt; |
There was a problem hiding this comment.
I would typically call this EmailMessageSys.
| } | ||
|
|
||
| pub async fn reply(&self, message: EmailMessage) -> Result<(), JsValue> { | ||
| JsFuture::from(self.0.reply(message.0)?).await?; |
There was a problem hiding this comment.
You may want to wrap these async calls as follows, otherwise the method's Future will not be Send:
let promise = self.0.reply(message.0)?;
let fut = SendFuture::new(JsFuture::from(promise));
fut.await?;Basically, you don't want to hold JsFuture across await boundary.
| unsafe impl Send for EmailMessage {} | ||
| unsafe impl Sync for EmailMessage {} | ||
|
|
||
| impl JsCast for EmailMessage { |
There was a problem hiding this comment.
I may be wrong, but you may not need all of this boilerplate for types that do not need to implement EnvBinding.
There was a problem hiding this comment.
Send and Sync are autotraits, you can test whether they are automatically implemented by adding a test such as
fn is_normal<T: Sized + Send + Sync + Unpin>() {}
#[test]
fn normal_types() {
is_normal::<EmailMessage>();
}
|
As this PR seemed to be dead, I've got a working implementation and would love some comments.(#715) |
4 participants
Closes #274
Adds Email Worker support in workers-rs in addition to Send Email support