@rgmz previously verification was based on range of 2xx Http status. Is there any particular reason to change it ?

Many of the detectors are based on a template with generic conditions. I think it's prudent to only consider something verified if it matches known/documented status codes.

Unexpected API/site changes, and thus status code changes, will propagate as an error rather than creating false-positives.

The API reference of OpenAI has well defined the error codes (also In there python library). In my opinion, Its good to parse the response but It wont make any effect on verification.

In my opinion, Its good to parse the response but It wont make any effect on verification.

I agree that parsing error codes doesn't make sense for OpenAI (although there are some APIs where 401 vs 403 is an importance difference).

I don't think it's a good practice to be liberal with what status codes are considered "valid" when only a specific one is expected. It's a recipe for false positives, imo.

@rgmz Metadata here is an additional information and the credentials are already been verified based on HTTP Status code. Base on the conversation we had on #2807 and #2808 , these are non-fatal errors and can be ignored without affecting the verification result.

..these are non-fatal errors and can be ignored without affecting the verification result.

The question I posed was actually whether it makes sense to return "verified" alongside an error to indicate a potential problem with the response. Generally speaking, I think it's much safer to handle errors rather than ignore them if err == nil { ... }.

There's a larger problem of surfacing errors and giving them precedence, but that's a different topic. e.g., the GitHub detector failing due to a DNS timeout and the JDBC detector failing because of an invalid hosts shouldn't be treated equally.

You are right that these errors should not be ignored. Unfortunately, right now, Detector result struct does not supports to hold these kind of errors (other than verification). In future, we can have separate error field(s) like @dustin-decker proposed to name it as EnrichmentError.

Unfortunately, right now, Detector result struct does not supports to hold these kind of errors (other than verification).

I would still consider them verification errors. e.g., invalid JSON can be indicative of false-positives or the API not behaving as expected (#2099).

Thanks @rgmz for sharing this case study. It is valid use case to convince me. One more case came in my mind is if an API started to redirected to valid page (with status 200) with HTML in response (may be home page), It will also be indicative of false-positives.