Missing credentials for scraper. CompanyId: hapoalim #268
Conversation
| if (!credentials || (!credentials.username && !credentials.num && !credentials.id) || !credentials.password) { | ||
| throw new Error(`Missing credentials for scraper. CompanyId: ${companyId}`); | ||
| } |
There was a problem hiding this comment.
I removed this. This is a complicated condition and its maintaining worth more than its value.
There was a problem hiding this comment.
suggesting to place the validation logic(s) at the input forms
There was a problem hiding this comment.
This validation was added because before we had it, the error in this case was cryptic.
You want to remove it because it fails too often in cases where there are credentials that are valid?
If this is the case, I suggest replacing the exception with a console
There was a problem hiding this comment.
suggesting to place the validation logic(s) at the input forms
That's a good idea. Doesn't have to block this PR.
Its actually a good first issue.
There was a problem hiding this comment.
You want to remove it because it fails too often in cases where there are credentials that are valid?
If this is the case, I suggest replacing the exception with a console
My point is it failed because of a bug in the code, and it is not so important to fail it earlier before the scraping.
I think it is very complicated logic for an almost useless use case. It is complicated because you need to validate the loginFields that differ in each scrapper.
suggesting to place the validation logic(s) at the input forms
And in any way, you can't insert an empty field through the UI.
| if (!credentials || (!credentials.username && !credentials.num && !credentials.id) || !credentials.password) { | ||
| throw new Error(`Missing credentials for scraper. CompanyId: ${companyId}`); | ||
| } |
There was a problem hiding this comment.
This validation was added because before we had it, the error in this case was cryptic.
You want to remove it because it fails too often in cases where there are credentials that are valid?
If this is the case, I suggest replacing the exception with a console
baruchiro
deleted the
baruchiro/Missing-credentials-for-scraper-CompanyId-hapoalim
branch
fix #257