department-of-veterans-affairs / notification-api

Notification API
MIT License
15 stars 9 forks source link

BUG: v3 Catch KeyError and Raise BadRequestError for Invalid phone_info #1585

Open cris-oddball opened 6 months ago

cris-oddball commented 6 months ago

Description

With the change to the API for a similar issue in v2 on #1525, we now return an error on POST sms with an invalid prefix. v3 however defers validation downstream and so we still get a long stacktrace. This error should be handled better for v3.

Steps to Reproduce

  1. send a POST v3/sms request with the following payload
    {
    "template_id": "{{sms-template-id}}",
    "phone_number": "+80888888888"
    }
  2. Observe that a 202 and a notification id is returned by the route, but GET v2/notifications/{notification-id} stays stuck at creating
  3. Observe the following logs Screenshot 2024-01-02 at 8.43.00 AM.jpg
Screenshot 2024-01-02 at 8.43.17 AM.jpg

Workaround

Is there something we can do to work around this issue in the meantime? None

Impact/Urgency

Poor experience, should be resolved before we release v3.

Expected Behavior

This is an engineering decision point since we are deferring certain validations. But this should not result in a long stacktrace in the logs and a status of 'creating'.

QA Considerations

Additional Info & Resources

mjones-oddball commented 6 months ago

Hey team! Please add your planning poker estimate with Zenhub @cris-oddball @edDocMe360 @EvanParish @k-macmillan @kalbfled