New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(backend): webhook max retry #2541

Merged

sabineschaller merged 38 commits into main from s2-webhook-retry

Mar 19, 2024

Member

sabineschaller commented Mar 14, 2024

Changes proposed in this pull request

adds env variable for webhook max retries
checks whether number of attempts < webhookMaxRetry before sending event

Context

fixes Set the maximum webhook retries #2464

Checklist


          feat(backend): webhook max retry

0c3aa49

sabineschaller requested review from melissahenderson and hajjimo as code owners

March 14, 2024 12:26

github-actions bot added type: tests pkg: backend type: source pkg: documentation type: documentation labels

netlify bot commented Mar 14, 2024 •

edited

✅ Deploy Preview for brilliant-pasca-3e80ec ready!

Name	Link
🔨 Latest commit	`e94fdb1`
🔍 Latest deploy log	https://app.netlify.com/sites/brilliant-pasca-3e80ec/deploys/65f9a64dd046b900087bba44
😎 Deploy Preview	https://deploy-preview-2541--brilliant-pasca-3e80ec.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.


          fix: tighten filter

1e13dc9

sabineschaller requested review from mkurapov and BlairCurrey

March 14, 2024 12:27

mkurapov reviewed

View reviewed changes

packages/documentation/src/content/docs/integration/deployment.md Outdated Show resolved Hide resolved

packages/backend/src/webhook/service.test.ts

@@ @@ -375,5 +387,13 @@ describe('Webhook Service', (): void => { @@
                       event.createdAt.getTime() + RETRY_BACKOFF_MS
                     )
                   })
+                  test('Does not send event if webhookMaxAttempts is reached', async (): Promise<void> => {

Contributor

mkurapov Mar 18, 2024

can we assert using nock that the webhook was not fired here? just as a sanity (in addition to asserting that processNext() returns undefined).

packages/backend/src/webhook/service.test.ts

@@ @@ -375,5 +387,13 @@ describe('Webhook Service', (): void => { @@
                       event.createdAt.getTime() + RETRY_BACKOFF_MS
                     )
                   })
+                  test('Does not send event if webhookMaxAttempts is reached', async (): Promise<void> => {

Contributor

mkurapov Mar 18, 2024

we can also use withConfigOverride function to make sure we never have to change this test in case the default config number changes.


          Update packages/documentation/src/content/docs/integration/deployment.md

4b73b40

Co-authored-by: Max Kurapov <max@interledger.org>

sabineschaller removed request for melissahenderson and hajjimo

March 19, 2024 08:59

sabineschaller and others added 14 commits

March 19, 2024 15:04


          test: address review feedback

09a979a


          chore: add feature requests to contributing.md (#2542)

3f1300b


          chore: enable graphql protection - maxDepth, blockFieldSuggestions, m…

c0b0974

…axCost (#2537)

* chore: add graphql depth restriction

* feat: properly enable graphql armor

* style: remove unnecessary space

* chore: add armor to auth admin server


          fix(deps): update module github.com/aws/aws-sdk-go to v1.50.38 (#2543)

cfa8d60

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>


          fix(deps): update dependency @interledger/open-payments to v6.7.2 (#2544

828241c

)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>


          chore(frontend): add X-Frame-Options header (#2538)

7ee9cbd


          chore(deps): update dependency @types/lodash to ^4.17.0 (#2546)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>


          chore(deps): update dependency @types/react to ^18.2.66 (#2545)

072a7e2

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>


          fix(deps): update dependency @interledger/open-payments to v6.8.0 (#2547

8840e4a

)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>


          fix(deps): update module github.com/aws/aws-sdk-go to v1.51.0 (#2548)

f4748d8

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>


          fix(deps): update dependency @interledger/docs-design-system to ^0.3.2 (

bd3e3d3

#2549)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>


          fix(deps): update dependency axios to v1.6.8 (#2556)

9a1796a

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>


          fix(deps): update dependency isbot to v5 (#2553)

2af8090

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>


          chore(deps): update typescript-eslint monorepo to v7 (#2552)

1d589c1

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

beniaminmunteanu and others added 8 commits

March 19, 2024 15:24


          feat(telemetry): LIVENET now points to new livenet NLB (#2523)

467c3be

* feat(telemetry): LIVENET now points to new livenet NLB

* Update packages/backend/src/config/app.ts

Co-authored-by: Max Kurapov <max@interledger.org>

* Update packages/backend/src/config/app.ts

Co-authored-by: Max Kurapov <max@interledger.org>

---------

Co-authored-by: Max Kurapov <max@interledger.org>
Co-authored-by: Sabine Schaller <sabine@interledger.org>


          feat(auth): improve GNAP error responses (#2400)

95aa007

* feat(auth): improve GNAP error responses

* fix: add extra arg to ctx.throw

* feat: middleware error improvements

* feat: add error messages to grant details endpoint

* fix: update error messages in grant routes

* feat: create utility function to generate GNAP errors, create enum for GNAP error codes

* feat: helper function throws instead of generates response


          feat: integration tests (#2380)

c972c36

* feat: setup basic test env

* feat: seed integration environment

- cmd to start integration environment and run tests
- seeds environment on test run
- extracts common MASE functionality into new mock-account-servicing-lib

* refactor(localenv,integration): move graphql url to config from seed

- moves graphql url to config from seed
- also removes self section which only contained unused properties

* refactor: move testenv configuration into new dir

* refactor: cleanup env vars

* feat: add webhook server, fix env vars

* chore: comment

* feat: start grant request test

* fix: eslint errors

* fix: docker compose env vars

* feat: add --build arg

* fix: incoming-payment grant initiation request

Grant request was failing with invalid signature. This was fixed by directing hte backend to use the same private key being used to create the open payments client.

* fix: ts error

* feat: implement tests through Create Quote

* chore: upgrade op client

* feat: partially implemented grant request outgoing payment test

* feat: rework to host.docker.internal

- switches hostname to host.docker.internal to resolve url mismatch problem
- finishes grant request outgoing payment test
- cleans up some configuration and test variables

* feat: add p2p flow test

* feat: add continuation step with consent mocking

* fix: rm obsolete type cast to any and comment

* feat: add create ougoing payment test

* fix: bad pnpm-lock merge

* feat: build deps in mock ase job

* feat: get non existant wallet address test

* fix: update open payments pkg

* chore: fix lint warnings

* feat: implement continuation polling

* chore: test cleanup

* refactor: generate gql in tests instead of import from lib

* chore: rm old comment

* chore: use latest http-singature-utils to match other deps

* chore: pnpm i

* chore: use latest koa-bodyparser

* chore: rm engine strict

* chore: revert rm engine strict

- fixes netlify ci failure but ultimately not the correct fix

* chore(integration): dont ignore env, rm example env

* refactor: use docker healthcheck for running tests

* chore: move healthcheck to last started docker container

* feat: use latest open payments pkg, no body requird on continuation

* chore: pnpm i, fix broken lockfile (no apollo version)

* refactor: move webhook event enum to types

* refactor: move run integration script to test/integration

* Update packages/mock-account-servicing-lib/package.json

Co-authored-by: Sabine Schaller <sabine@interledger.org>

* refactor: simplify mock-account-servicing-entity ci step

* feat(integration): exit run-tests early if containers fail to start

* feat: use pino logger

* refactor: rename mock account servicing lib

* refactor: rename class files to camel case

* fix: import correct body parser

* refactor: poll instead of delay

* refactor: simplify call to poll condition only

* fix: update filenames

* refactor: change filename name to kebab case

---------

Co-authored-by: Sabine Schaller <sabine@interledger.org>


          fix(deps): update dependency isbot to ^5.1.2 (#2567)

663a33c

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>


          chore(deps): update dependency @types/react to ^18.2.67 (#2566)

ddeca41

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>


          fix(deps): update module github.com/aws/aws-sdk-go to v1.51.2 (#2565)

64c2b10

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>


          chore(deps): update helm release redis to v18.19.4 (#2570)

0b390ec

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>


          feat(backend): make SPSP optional (#2560)

9e27aea

* feat: flag to enable/disable SPSP

* test: update to cover all cases

* docs: add link to glossary

github-actions bot added type: ci pkg: frontend pkg: auth type: localenv pkg: mock-ase labels


          chore: fix lockfile

18f48c5

mkurapov reviewed

View reviewed changes

packages/backend/src/webhook/service.test.ts Outdated

@@ @@ -389,11 +390,18 @@ describe('Webhook Service', (): void => { @@
                   })
                   test('Does not send event if webhookMaxAttempts is reached', async (): Promise<void> => {
+                    const scope = nock(webhookUrl.origin)
                     await event.$query(knex).patch({
                       attempts: 10

Contributor

mkurapov Mar 19, 2024

maybe now attempts: Config.webhookMaxRetry is better

packages/backend/src/webhook/service.test.ts Outdated

Comment on lines 399 to 403

+                    expect(
+                      nock.emitter.on('no match', (_req) => {
+                        return true
+                      })
+                    ).toBeTruthy()

Contributor

mkurapov Mar 19, 2024

This will always be truthy since nock.emitter.on just returns an EventEmitter. Also, I think this event listener would have to be defined before we make any requests for this to work.

Maybe instead of doing this, we could just piggyback off of nock(webhookUrl.origin) as such:

      let requests = 0
      const scope = nock(webhookUrl.origin)
        .post('/')
        .reply(200, () => {
          requests++
        })
      expect(requests).toBe(0)

Member Author

sabineschaller Mar 19, 2024

This is what happens when you try to finish things quickly...

sabineschaller added 5 commits

March 19, 2024 15:35


          test: address feedback

a51f460


          delete lockfile

33dc83b


          add lockfile

ae7138f


          delete lockfile

93c9f2a


          Merge branch 'main' into s2-webhook-retry

6ae1c70

github-actions bot removed type: ci pkg: frontend pkg: auth type: localenv pkg: mock-ase labels

sabineschaller added 2 commits

March 19, 2024 15:48


          fix lockfile this time?

2ad8eea


          fix: formatting

e94fdb1

mkurapov approved these changes

View reviewed changes

sabineschaller merged commit b124abd into main

22 checks passed

sabineschaller deleted the s2-webhook-retry branch

March 19, 2024 16:03

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment