GoodDollar / GoodServer

Backend to support the GoodDAPP
MIT License
13 stars 13 forks source link

(Bug) ongage errors #364

Closed sirpy closed 2 years ago

sirpy commented 2 years ago

Description

Looking at the logs (regex /OnGage error/), we have two repeating errors

Todo

johnsmith-gooddollar commented 2 years ago
  1. added retry to OnGage calls: https://github.com/GoodDollar/GoodServer/commit/e43eeffb70d62141e3705bab8ad814030f746cf0
  2. calls failing with 404 are existence or duplicates checks: https://github.com/GoodDollar/GoodServer/blob/master/src/server/crm/ongage.js#L134 https://github.com/GoodDollar/GoodServer/blob/master/src/server/crm/ongage.js#L85
sirpy commented 2 years ago

@johnsmith-gooddollar

  1. you didnt explain why the call fails. why does getting user by email throws an exception?
  2. and? does it hurt current logic? is it ok that it throws an exception where it does?
sirpy commented 2 years ago

@johnsmith-gooddollar that retry commit is pretty big, did you test things?

sirpy commented 2 years ago

@johnsmith-gooddollar examine the 404 errors and trace why they happen and suggest fix

johnsmith-gooddollar commented 2 years ago

@johnsmith-gooddollar examine the 404 errors and trace why they happen and suggest fix

@sirpy

I've already examined Those errors are ok they are part of our logic (see my comment above). So no fix is needed.

To determine if some email exists/users we're doing call get by email If it fails with 404 - that means no email exists/email not used

I've linked exact place in the code containing this logic

Logging comes from crm api class which logs any success/failed intake response

sirpy commented 2 years ago

@johnsmith-gooddollar what about the refactor you did... did you verify things are working? can you push it to production?

johnsmith-gooddollar commented 2 years ago

@johnsmith-gooddollar what about the refactor you did... did you verify things are working? can you push it to production?

  1. I've tsted locally
  2. @vldkh is performing QA check now
  3. I'll also add unit test for retry case today
johnsmith-gooddollar commented 2 years ago

@sirpy

a) it was tested on QA, everything is fine b) i've just added unit test for retry case, found one issue happening after retry, fixed it, pushed to master and staging https://github.com/GoodDollar/GoodServer/commit/c3f4cb94cbcfef6e2ba44dea57ffe93871ff7db2

Now we could push to PROD (with the wallet release)