From d2d9cb369f54da6235e672d1ed3ed27c1063aa0c Mon Sep 17 00:00:00 2001 From: Michael Wellman Date: Wed, 4 Dec 2024 14:23:47 -0500 Subject: [PATCH 1/2] #2070 Retrying Twilio 400 Errors (#2152) --- app/clients/sms/twilio.py | 8 ++++++- tests/app/clients/test_twilio.py | 36 ++++++++++++++++++++++++++------ 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/app/clients/sms/twilio.py b/app/clients/sms/twilio.py index 9e82868501..81f57e873e 100644 --- a/app/clients/sms/twilio.py +++ b/app/clients/sms/twilio.py @@ -58,6 +58,7 @@ class TwilioSMSClient(SmsClient): '21610': TwilioStatus(21610, NOTIFICATION_PERMANENT_FAILURE, OPT_OUT_MESSAGE), '21612': TwilioStatus(21612, NOTIFICATION_PERMANENT_FAILURE, 'Invalid to/from combo'), '21614': TwilioStatus(21614, NOTIFICATION_PERMANENT_FAILURE, 'Non-mobile number'), + '21617': TwilioStatus(21617, NOTIFICATION_PERMANENT_FAILURE, 'Message too long'), '21635': TwilioStatus(21635, NOTIFICATION_PERMANENT_FAILURE, 'Non-mobile number'), '30001': TwilioStatus(30001, NOTIFICATION_TEMPORARY_FAILURE, 'Queue overflow'), '30002': TwilioStatus(30002, NOTIFICATION_PERMANENT_FAILURE, 'Account suspended'), @@ -175,7 +176,6 @@ def send_sms( Return: a string containing the Twilio message.sid """ - start_time = monotonic() from app.dao.service_sms_sender_dao import ( dao_get_service_sms_sender_by_service_id_and_number, @@ -238,6 +238,12 @@ def send_sms( if e.status == 400 and 'phone number' in e.msg: self.logger.exception('Twilio send SMS request for %s failed', reference) raise InvalidProviderException from e + elif e.status == 400 and e.code == 21617: # Twilio error code for max length exceeded + self.logger.exception( + 'Twilio send SMS request for %s failed, message content max length exceeded.', reference + ) + self.logger.debug('Twilio error details for %s - %s: %s', reference, e.code, e.msg) + raise NonRetryableException('Twilio request failed') from e else: raise except: diff --git a/tests/app/clients/test_twilio.py b/tests/app/clients/test_twilio.py index 6544efe60d..9e86ebeef2 100644 --- a/tests/app/clients/test_twilio.py +++ b/tests/app/clients/test_twilio.py @@ -599,19 +599,43 @@ def test_send_sms_twilio_callback( assert response_dict['sid'] == twilio_sid -def test_send_sms_raises_invalid_provider_error_with_invalid_twilio_number( +@pytest.mark.parametrize( + ( + 'response_dict', + 'exception', + ), + ( + ( + { + 'code': 21606, + 'message': "The 'From' phone number provided (+61412345678) is not a valid message-capable Twilio phone number for this destination", + }, + InvalidProviderException, + ), + ( + { + 'code': 21617, + 'message': 'Unable to create record: The concatenated message body exceeds the 1600 character limit', + }, + NonRetryableException, + ), + ), + ids=( + 'invalid-from-number', + 'message-too-long', + ), +) +def test_send_sms_raises_non_retryable_exception_with_invalid_request( notify_api, mocker, + response_dict, + exception, ): to = '+61412345678' content = 'my message' reference = 'my reference' - response_dict = { - 'code': 21606, - 'message': "The 'From' phone number provided (+61412345678) is not a valid message-capable Twilio phone number for this destination", - } - with pytest.raises(InvalidProviderException): + with pytest.raises(exception): with requests_mock.Mocker() as r_mock: r_mock.post( f'https://api.twilio.com/2010-04-01/Accounts/{twilio_sms_client._account_sid}/Messages.json', From b6bc1e93937d3827c7cdbba485a957a069775a31 Mon Sep 17 00:00:00 2001 From: Kyle MacMillan <16893311+k-macmillan@users.noreply.github.com> Date: Wed, 4 Dec 2024 14:26:19 -0500 Subject: [PATCH 2/2] #2157 - Updated Github Action runs-on (#2158) --- .github/workflows/cd-pipeline.yml | 8 ++++---- .github/workflows/create-and-post-tag.yml | 2 +- .github/workflows/create-release-notes.yml | 2 +- .github/workflows/db-downgrade.yml | 2 +- .github/workflows/dependency-ticket-creation.yml | 2 +- .github/workflows/deploy-release.yml | 4 ++-- .github/workflows/deploy_lambda_dev.yml | 2 +- .github/workflows/deployment.yml | 8 ++++---- .github/workflows/dev-tests.yml | 4 ++-- .github/workflows/dev_deploy.yml | 4 ++-- .github/workflows/lambda-functions.yml | 2 +- .github/workflows/load-test-email-delivery-time.yaml | 2 +- .github/workflows/load-test-sms-response-time.yaml | 2 +- .github/workflows/pr-label-semver.yml | 2 +- .github/workflows/pre-tag-summary.yml | 2 +- .github/workflows/publish-release-notes.yml | 2 +- .github/workflows/run-regression.yml | 2 +- .github/workflows/twistlock.yml | 2 +- .github/workflows/update-datadog-image.yaml | 2 +- 19 files changed, 28 insertions(+), 28 deletions(-) diff --git a/.github/workflows/cd-pipeline.yml b/.github/workflows/cd-pipeline.yml index cd0898de4a..0c3969080f 100644 --- a/.github/workflows/cd-pipeline.yml +++ b/.github/workflows/cd-pipeline.yml @@ -20,7 +20,7 @@ on: jobs: prepare-deployment: - runs-on: ubuntu-latest + runs-on: ${{ vars.RUNS_ON }} steps: - name: Checkout code uses: actions/checkout@v4 @@ -34,7 +34,7 @@ jobs: needs: prepare-deployment environment: name: perf-deploy - runs-on: ubuntu-latest + runs-on: ${{ vars.RUNS_ON }} steps: - name: Pause for manual approval run: | @@ -53,7 +53,7 @@ jobs: build-push-artifacts: needs: create-and-post-tag - runs-on: ubuntu-latest + runs-on: ${{ vars.RUNS_ON }} steps: - name: Checkout Repo uses: actions/checkout@v4 @@ -80,7 +80,7 @@ jobs: needs: deploy-to-perf environment: name: staging-deploy - runs-on: ubuntu-latest + runs-on: ${{ vars.RUNS_ON }} steps: - name: Pause for manual approval run: echo "Deployment paused for manual approval to staging and production." diff --git a/.github/workflows/create-and-post-tag.yml b/.github/workflows/create-and-post-tag.yml index facea29d83..a339c864d8 100644 --- a/.github/workflows/create-and-post-tag.yml +++ b/.github/workflows/create-and-post-tag.yml @@ -12,7 +12,7 @@ on: jobs: create-and-post-tag: - runs-on: ubuntu-latest + runs-on: ${{ vars.RUNS_ON }} outputs: previousVersion: ${{ steps.create_tag.outputs.previousVersion }} newVersion: ${{ steps.create_tag.outputs.newVersion }} diff --git a/.github/workflows/create-release-notes.yml b/.github/workflows/create-release-notes.yml index 1718060059..1a767e10c4 100644 --- a/.github/workflows/create-release-notes.yml +++ b/.github/workflows/create-release-notes.yml @@ -13,7 +13,7 @@ on: jobs: create-release-notes: - runs-on: ubuntu-latest + runs-on: ${{ vars.RUNS_ON }} outputs: draftReleaseReference: ${{ steps.create_notes.outputs.draftReleaseReference }} permissions: diff --git a/.github/workflows/db-downgrade.yml b/.github/workflows/db-downgrade.yml index c27254baf6..d5a4f43963 100644 --- a/.github/workflows/db-downgrade.yml +++ b/.github/workflows/db-downgrade.yml @@ -12,7 +12,7 @@ on: default: container_tag jobs: run-downgrade: - runs-on: ubuntu-latest + runs-on: ${{ vars.RUNS_ON }} steps: - uses: actions/checkout@v4 diff --git a/.github/workflows/dependency-ticket-creation.yml b/.github/workflows/dependency-ticket-creation.yml index 6c52353021..3bcb1f1c90 100644 --- a/.github/workflows/dependency-ticket-creation.yml +++ b/.github/workflows/dependency-ticket-creation.yml @@ -6,7 +6,7 @@ on: jobs: create_issue: - runs-on: ubuntu-latest + runs-on: ${{ vars.RUNS_ON }} steps: - name: Checkout code uses: actions/checkout@v3 diff --git a/.github/workflows/deploy-release.yml b/.github/workflows/deploy-release.yml index d30b74a255..8c93364f8c 100644 --- a/.github/workflows/deploy-release.yml +++ b/.github/workflows/deploy-release.yml @@ -48,7 +48,7 @@ env: jobs: setup-environment: name: 'setup-env' - runs-on: ubuntu-latest + runs-on: ${{ vars.RUNS_ON }} steps: - name: Checkout Repo uses: actions/checkout@v4 @@ -70,7 +70,7 @@ jobs: upload-env-file: needs: [setup-environment] - runs-on: ubuntu-latest + runs-on: ${{ vars.RUNS_ON }} steps: - name: Checkout Repo uses: actions/checkout@v4 diff --git a/.github/workflows/deploy_lambda_dev.yml b/.github/workflows/deploy_lambda_dev.yml index 52f93fbdfd..6bc2ef81c9 100644 --- a/.github/workflows/deploy_lambda_dev.yml +++ b/.github/workflows/deploy_lambda_dev.yml @@ -36,7 +36,7 @@ on: jobs: setup_job: - runs-on: ubuntu-latest + runs-on: ${{ vars.RUNS_ON }} steps: - name: Checkout Repo uses: actions/checkout@v4 diff --git a/.github/workflows/deployment.yml b/.github/workflows/deployment.yml index f245b10b56..d427bd6e0a 100644 --- a/.github/workflows/deployment.yml +++ b/.github/workflows/deployment.yml @@ -14,7 +14,7 @@ on: jobs: run-migrations: - runs-on: ubuntu-latest + runs-on: ${{ vars.RUNS_ON }} steps: - uses: actions/checkout@v4 with: @@ -58,7 +58,7 @@ jobs: bash ./scripts/run_ci_migrations.sh -c ${{ inputs.environment }}-notification-cluster -e ${{ inputs.environment }} -t ${{ steps.register.outputs.arn }} deploy-api: - runs-on: ubuntu-latest + runs-on: ${{ vars.RUNS_ON }} needs: [run-migrations] steps: - uses: actions/checkout@v4 @@ -102,7 +102,7 @@ jobs: wait-for-service-stability: true deploy-celery: - runs-on: ubuntu-latest + runs-on: ${{ vars.RUNS_ON }} needs: [run-migrations] steps: - uses: actions/checkout@v4 @@ -146,7 +146,7 @@ jobs: wait-for-service-stability: true deploy-celery-beat: - runs-on: ubuntu-latest + runs-on: ${{ vars.RUNS_ON }} needs: [run-migrations] steps: - uses: actions/checkout@v4 diff --git a/.github/workflows/dev-tests.yml b/.github/workflows/dev-tests.yml index ff2003e1de..c789214820 100644 --- a/.github/workflows/dev-tests.yml +++ b/.github/workflows/dev-tests.yml @@ -64,7 +64,7 @@ jobs: make test code-scan: - runs-on: ubuntu-latest + runs-on: ${{ vars.RUNS_ON }} steps: - uses: actions/checkout@v4 with: @@ -75,7 +75,7 @@ jobs: - run: make check-vulnerabilities dependency-scan: - runs-on: ubuntu-latest + runs-on: ${{ vars.RUNS_ON }} steps: - uses: actions/checkout@v4 with: diff --git a/.github/workflows/dev_deploy.yml b/.github/workflows/dev_deploy.yml index 1fa362d61a..73f8b9ac1b 100644 --- a/.github/workflows/dev_deploy.yml +++ b/.github/workflows/dev_deploy.yml @@ -39,7 +39,7 @@ on: jobs: setup-environment: name: "setup-env-${{ inputs.environment }}" - runs-on: ubuntu-latest + runs-on: ${{ vars.RUNS_ON }} outputs: git-hash: ${{ steps.set-hash.outputs.commit-hash }} steps: @@ -61,7 +61,7 @@ jobs: build-push-artifacts: needs: [setup-environment] - runs-on: ubuntu-latest + runs-on: ${{ vars.RUNS_ON }} steps: - name: Checkout Repo uses: actions/checkout@v4 diff --git a/.github/workflows/lambda-functions.yml b/.github/workflows/lambda-functions.yml index 5e1ef09b5d..296dce57ef 100644 --- a/.github/workflows/lambda-functions.yml +++ b/.github/workflows/lambda-functions.yml @@ -29,7 +29,7 @@ env: jobs: # All of the lambda functions use layers for dependencies, if any. Bundling dependencies should not be necessary. build-and-deploy-lambda-functions: - runs-on: ubuntu-latest + runs-on: ${{ vars.RUNS_ON }} defaults: run: working-directory: "./lambda_functions" diff --git a/.github/workflows/load-test-email-delivery-time.yaml b/.github/workflows/load-test-email-delivery-time.yaml index f6654b55dd..6a75a9a068 100644 --- a/.github/workflows/load-test-email-delivery-time.yaml +++ b/.github/workflows/load-test-email-delivery-time.yaml @@ -46,7 +46,7 @@ on: jobs: trigger: - runs-on: ubuntu-latest + runs-on: ${{ vars.RUNS_ON }} steps: - uses: actions/checkout@v4 diff --git a/.github/workflows/load-test-sms-response-time.yaml b/.github/workflows/load-test-sms-response-time.yaml index 70f3d1c593..ed66e14848 100644 --- a/.github/workflows/load-test-sms-response-time.yaml +++ b/.github/workflows/load-test-sms-response-time.yaml @@ -56,7 +56,7 @@ on: jobs: trigger: - runs-on: ubuntu-latest + runs-on: ${{ vars.RUNS_ON }} steps: - uses: actions/checkout@v4 diff --git a/.github/workflows/pr-label-semver.yml b/.github/workflows/pr-label-semver.yml index 70ea3a6f45..84a0184b90 100644 --- a/.github/workflows/pr-label-semver.yml +++ b/.github/workflows/pr-label-semver.yml @@ -5,7 +5,7 @@ on: jobs: check-pr-sem: - runs-on: ubuntu-latest + runs-on: ${{ vars.RUNS_ON }} permissions: contents: write steps: diff --git a/.github/workflows/pre-tag-summary.yml b/.github/workflows/pre-tag-summary.yml index c6304407e3..1868bde3f0 100644 --- a/.github/workflows/pre-tag-summary.yml +++ b/.github/workflows/pre-tag-summary.yml @@ -5,7 +5,7 @@ on: jobs: pre-tag-summary: - runs-on: ubuntu-latest + runs-on: ${{ vars.RUNS_ON }} permissions: contents: write steps: diff --git a/.github/workflows/publish-release-notes.yml b/.github/workflows/publish-release-notes.yml index 02af85f30f..10fca4e695 100644 --- a/.github/workflows/publish-release-notes.yml +++ b/.github/workflows/publish-release-notes.yml @@ -8,7 +8,7 @@ on: type: string jobs: publish-release-notes: - runs-on: ubuntu-latest + runs-on: ${{ vars.RUNS_ON }} permissions: contents: write steps: diff --git a/.github/workflows/run-regression.yml b/.github/workflows/run-regression.yml index 98c17cc08a..d59269bd0f 100644 --- a/.github/workflows/run-regression.yml +++ b/.github/workflows/run-regression.yml @@ -22,7 +22,7 @@ on: jobs: trigger-regression-tests: name: "Run regression for ${{ inputs.environment }}" - runs-on: ubuntu-latest + runs-on: ${{ vars.RUNS_ON }} environment: ${{ inputs.environment }} steps: - uses: actions/checkout@v4 diff --git a/.github/workflows/twistlock.yml b/.github/workflows/twistlock.yml index 30f67e2326..9149e4386d 100644 --- a/.github/workflows/twistlock.yml +++ b/.github/workflows/twistlock.yml @@ -9,7 +9,7 @@ on: jobs: twistlock-scan: - runs-on: ubuntu-latest + runs-on: ${{ vars.RUNS_ON }} steps: - uses: actions/checkout@v4 with: diff --git a/.github/workflows/update-datadog-image.yaml b/.github/workflows/update-datadog-image.yaml index dc0c64ede1..a58f0ede60 100644 --- a/.github/workflows/update-datadog-image.yaml +++ b/.github/workflows/update-datadog-image.yaml @@ -14,7 +14,7 @@ on: jobs: update-image: - runs-on: ubuntu-latest + runs-on: ${{ vars.RUNS_ON }} env: IMAGE_REPOSITORY: "datadog/agent" IMAGE_TAG: ${{ github.event.inputs.version_tag }} # Use the provided version tag for the image