Chapter 11.8.3.3 Idempotency

[lenoqt]

Can anybody help me to understand why the body saved is empty, leading to a failed idempotency test?

#[tracing::instrument(
name = "Publish a newsletter issue",
skip(form, pool, email_client, user_id),
fields(user_id=%*user_id)
)]
pub async fn publish_newsletter(
    form: web::Form<NewsletterContent>,
    user_id: ReqData<UserId>,
    pool: web::Data<PgPool>,
    email_client: web::Data<EmailClient>,
) -> Result<HttpResponse, actix_web::Error> {
    let user_id = user_id.into_inner();
    let NewsletterContent {
        title,
        text_content,
        html_content,
        idempotency_key,
    } = form.0;
    let idempotency_key: IdempotencyKey = idempotency_key.try_into().map_err(e400)?;
    // Return early if we have a saved response in the database
    if let Some(saved_response) = get_saved_response(&pool, &idempotency_key, *user_id)
        .await
        .map_err(e500)?
    {
        return Ok(saved_response);
    }
    let subscribers = get_confirmed_subscribers(&pool).await.map_err(e500)?;
    for subscriber in subscribers {
        // The compiler forces us to handle both cases `Ok` & `Err`
        match subscriber {
            Ok(subscriber) => {
                email_client
                    .send_email(&subscriber.email, &title, &html_content, &text_content)
                    .await
                    // Use `with_context` when there's a runtime cost. format macro stores the string into
                    // the heap, using `context` would be allocating that string every time we send an
                    // email out.
                    .with_context(|| {
                        format!("Failed to send newsletter issue to {}", subscriber.email)
                    })
                    .map_err(e500)?;
            }
            Err(error) => {
                tracing::warn!(
                    // We record the error chain as a structured field
                    // on the log record.
                    error.cause_chain = ?error,
                    error.message = %error,
                    "Skipping a confirmed subscriber. Their stored contact details are invalid"
                );
            }
        }
    }
    FlashMessage::info("The newsletter issue has been published!").send();
    let response = see_other("/admin/newsletters");
    let response = save_response(&pool, &idempotency_key, *user_id, response)
        .await
        .map_err(e500)?;
    Ok(response)
}

[LukeMathWalker]

Difficult to say based on this snippet alone. Can you upload your code to a public repository and link it here?

[lenoqt]

Difficult to say based on this snippet alone. Can you upload your code to a public repository and link it here?

Sure, here it is. It is sightly different since sometimes didn't had the book in hand and also I'm using SendGrid.

https://github.com/lenoqt/zero2prod/tree/ch11833idempotency

[chyczewski-maciej]

I believe the empty body is to be expected in this case. The happy path (either the first call or the other calls with the same idempotency key) is meant to return a response that redirects you to /admin/newsletter, so there's no need for a body.

Are you sure it fails because the body is empty? I think there's an alternative reason why it might fail - missing the flash message on the subsequent GET /admin/newsletters call in the test.

I think the FlashMessage set-cookie header is not part of the response object when it's stored using save_response call. I think it's set in the middleware after the response is returned from the publish_newsletter.

If these headers were not to be stored into the database then they would not be included in the "return early" response based on the data stored in db. Then you get a successful redirection but without the flash message cookie set and the subsequent call to /admin/newsletters would not render the flash the message and that's one of the assertions in the newsletter_creation_is_idempotent tests in the book

[chyczewski-maciej]

I've searched in the old issues reported. Turns out somebody has reported an issue in the same section of code. Similar to what I've just described https://github.com/LukeMathWalker/zero-to-production/issues/165

[lenoqt]

Checking what it is being saved, effectively is the 303 which the body is empty and not the subsequent redirect, which is expected from see_other, is this fixed in the new version of the book?

What confuses me more is that implementing try_processing and NextAction fixes this issue, but I cannot understand how and what is missed on the headers.

[lenoqt]

Reading back your analysis @chyczewski-maciej and #165 I see that the chapter was missing sending the FlashMessage after get_saved_response and returning.

#[tracing::instrument(
name = "Publish a newsletter issue",
skip(form, pool, email_client, user_id),
fields(user_id = % * user_id)
)]
pub async fn publish_newsletter(
    form: web::Form<NewsletterContent>,
    user_id: ReqData<UserId>,
    pool: web::Data<PgPool>,
    email_client: web::Data<EmailClient>,
) -> Result<HttpResponse, actix_web::Error> {
    let NewsletterContent {
        title,
        text_content,
        html_content,
        idempotency_key,
    } = form.0;
    let idempotency_key: IdempotencyKey = idempotency_key.try_into().map_err(e400)?;
    if let Some(saved_response) = get_saved_response(&pool, &idempotency_key, **user_id)
        .await
        .map_err(e500)?
    {
        FlashMessage::info("The newsletter issue has been published!").send();
        return Ok(saved_response);
    }
    let subscribers = get_confirmed_subscribers(&pool).await.map_err(e500)?;
    for subscriber in subscribers {
        // The compiler forces us to handle both cases `Ok` & `Err`
        match subscriber {
            Ok(subscriber) => {
                email_client
                    .send_email(&subscriber.email, &title, &html_content, &text_content)
                    .await
                    // Use `with_context` when there's a runtime cost. format macro stores the string into
                    // the heap, using `context` would be allocating that string every time we send an
                    // email out.
                    .with_context(|| {
                        format!("Failed to send newsletter issue to {}", subscriber.email)
                    })
                    .map_err(e500)?;
            }
            Err(error) => {
                tracing::warn!(
                    // We record the error chain as a structured field
                    // on the log record.
                    error.cause_chain = ?error,
                    // Using `\` to split a long string literal over
                    // two lines, without creating a `\n` character.
                    "Skipping a confirmed subscriber. \
                    Their stored contact details are invalid"
                )
            }
        }
    }
    FlashMessage::info("The newsletter issue has been published!").send();
    let response = see_other("/admin/newsletters");
    let response = save_response(&pool, &idempotency_key, **user_id, response)
        .await
        .map_err(e500)?;
    Ok(response)
}

This fixes the issue but it is better just to have as in ch11

fn success_message() -> FlashMessage {
    FlashMessage::info("The newsletter issue has been published!")
}