Skip to content

Unusable discount code returns 500 "An internal error occurred" instead of 400 #385

Description

@v0l

Symptom

Applying a discount code that does not exist (or is expired, inactive, or otherwise unusable) at checkout returns HTTP 500 with the body {"error":"An internal error occurred"}, instead of a 400 carrying the message the engine actually wrote. The customer sees "An internal error occurred" under the discount field.

Reproduce: GET /api/v1/subscriptions/{id}/renew/quote?method=lightning&code=DOESNOTEXIST (same for /api/v1/vm/{id}/renew and the subscription renew endpoint).

Cause

PricingEngine::quote_discount rejects with an untyped anyhow!:

  • lnvps_api_common/src/discount/engine.rs:114bail!("Discount code is not valid for this order")
  • lnvps_api_common/src/discount/engine.rs:129let refuse = || anyhow!("Discount code is not valid for this order"); used for every DB-guard rejection (unknown code, wrong company, inactive, outside the validity window, usage/per-user limit exhausted)

The handler converts that through the blanket impl in lnvps_api_common/src/routes.rs:165:

impl From<anyhow::Error> for ApiError {
    fn from(value: anyhow::Error) -> Self {
        if let Some(cap) = value.downcast_ref::<crate::CapacityError>() {
            return Self::conflict(cap);
        }
        Self::internal(value)
    }
}

CapacityError is the only error type with a downcast arm, so the discount error falls through to ApiError::internal, which logs it and — with verbose_internal_errors() off, as in production — discards the message and returns 500 "An internal error occurred".

So the deliberately-generic-but-useful message never reaches the customer, and a routine client mistake is logged as an internal error.

Suggested fix

Mirror CapacityError: give the discount engine a typed error (e.g. DiscountError::NotUsable) implementing std::error::Error, return it from quote_discount / discount_candidates, and add a downcast arm mapping it to ApiError::bad_request.

This keeps the single generic message for every rejection reason — the enumeration-resistance property the code comments call out is deliberate and should be preserved — while making the response a 400 that says what went wrong.

Related

While reading this: get_discount_by_code is SELECT * FROM discount WHERE code = ? (lnvps_db/src/mysql.rs:4130), and the engine's unit tests assert save10 is rejected where SAVE10 succeeds. Whether a customer typing a code in lower case works therefore depends on the discount.code column collation. If it is case-sensitive in production, that is worth normalising (or documenting), since published codes get typed however people feel like typing them.

Metadata

Metadata

Assignees

No one assigned

    Labels

    apiUser-facing or admin API changesbugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions