search

change-metrics / monocle

Monocle helps teams and individual to better organize daily duties and to detect anomalies in the way changes are produced and reviewed.
https://demo.changemetrics.io/
GNU Affero General Public License v3.0
362 stars 56 forks source link

Handle Authentication Error #1062

Closed TristanCacqueray closed 9 months ago

TristanCacqueray commented 9 months ago

This change avoids retrying requests that fail with a 401 error. #1060

TristanCacqueray commented 9 months ago

@morucci I've replaced the streaming error handling with a simpler approach in this PR. Though I don't expect it to fix the issue, and if it does, it would be good to know why. What we observed is that for take datas, the processing ends correctly:

2023-09-19 17:07:53 WARNING Lentille.GraphQL:235: Could not fetch the current rate limit {"index":"monocle2","crawler":"gc-monocle","stream":"TaskDatas"}
2023-09-19 17:07:53 WARNING Macroscope.Worker:132: Error occured when consuming the document stream ...

But for projects and pull-requests, the error is somehow ignored:

2023-09-19 17:08:03 WARNING Lentille.GraphQL:235: Could not fetch the current rate limit {"index":"monocle2","crawler":"gc-monocle","stream":"Changes"}
2023-09-19 17:08:03 INFO    Macroscope.Worker:188: Continuing on next entity {"index":"monocle2","crawler":"gc-monocle","stream":"Changes"}
TristanCacqueray commented 9 months ago

@morucci in the last commit I've also added a test to validate the logic, and the test pass with or without the refactor, so I think the issue is somewhere else in the GraphQL module.

morucci commented 9 months ago

Thanks for the refactor. Let's merge it. However yes it is a weird that processStream do not receive the GraphError from streamFetch.