0 results due to server error

[ryuusama09]

PR Description

Solving (#5661 )
This PR tries to resolve the ambiguity caused by 0 results caused by no matching response from server vs server error resulting in no response. The main motive is to return the error code from the scraper to get the context that there has been a server error which is difficult to reproduce it manually.

[ryuusama09]

@nabobalis , have drafted a tentative pr showing what changes i mean to do . The responses are too plain and generic , we can refine this , while also trying to not miss out on some edge cases . Currently Ive submitted the code for not giving partial response incase of server failure . We can change it easily , the context here is to give you clarity on what is being done

[nabobalis]

@nabobalis , have drafted a tentative pr showing what changes i mean to do . The responses are too plain and generic , we can refine this , while also trying to not miss out on some edge cases . Currently Ive submitted the code for not giving partial response incase of server failure . We can change it easily , the context here is to give you clarity on what is being done

I am not sure that returning a string makes much sense, how does that interact with a unifiedresposne?

[nabobalis]

Would len(results) also work?

[ryuusama09]

Nope , len (results) can be 1 , 0 or 2 depending on the client queried . I found it out yesterday while testing it for different clients

[nabobalis]

How does this look to a user? Do you have examples of a user query and the return/output when you hit this code?

[ryuusama09]

How does this look to a user? Do you have examples of a user query and the return/output when you hit this code?

right now i return this plain reply

It can be improved . I tried wrapping it in a unifiedresponse , but i failed to do so.. This is because currently it wraps
queryresponseRow , queryresponseColumn and table if i am not wrong

[nabobalis]

I think it has to work with a unifiedresponse.

I would still first replicate a server error and then work out how what happens to a query.

[ryuusama09]

How does this look to a user? Do you have examples of a user query and the return/output when you hit this code?

right now i return this plain reply It can be improved . I tried wrapping it in a unifiedresponse , but i failed to do so.. This is because currently it wraps queryresponseRow , queryresponseColumn and table if i am not wrong

I would still first replicate a server error and then work out how what happens to a query.

I think it has to work with a unifiedresponse.

Then we can add a warning attribute to it and return it as a warning string ?!.

I would still first replicate a server error and then work out how what happens to a query.

Hmm , any suggestions on how can i replicate it ?

[nabobalis]

I think it has to work with a unifiedresponse.

Then we can add a warning attribute to it and return it as a warning string ?!.

Well a warning attribute might not be a bad idea, we have something similar with the fetch return as .errors. I don't think it should just be a string tho.

But this should only be for an error/warning. IF there are no results, an empty response seems to make the most sense to me.

I would still first replicate a server error and then work out how what happens to a query.

Hmm , any suggestions on how can i replicate it ?

I don't have anything to mind.

[ryuusama09]

But this should only be for an error/warning. IF there are no results, an empty response seems to make the most sense to me.

Yes , an empty response along with warning right ? (because the original issue mentioned to raise a warning in case of 0 matched results)

[nabobalis]

I personally don't see the need for a warning if the server was fine but there are no results

[ryuusama09]

I personally don't see the need for a warning if the server was fine but there are no results

So this simplifies it , i'll raise a warning only if server gives error. right now i've kept max re-enqueueing limit as 5 . So we can make a warning attribute in unifiedResponse. what should the warning look like ?

[nabobalis]

What about for the other clients that don't use the scraper?

[ryuusama09]

What about for the other clients that don't use the scraper?

As of now , I was testing it using scraper dependent clients , for others clients I believe there wont be much change . (like the overall procedure could be the same .
for ex : we will add warnings to JSOCresponse similar to UnifiedResponse )

[ryuusama09]

What about for the other clients that don't use the scraper?

Sorry for the long break @nabobalis . I think , for those clients that dont use the generic client , we can implement the same logic similarly in the baseclient . Although , can I ask why few clients are based on genericclient , while others on base client ?

[nabobalis]

What about for the other clients that don't use the scraper?

Sorry for the long break @nabobalis . I think , for those clients that dont use the generic client , we can implement the same logic similarly in the baseclient .

Can we?

Although , can I ask why few clients are based on genericclient , while others on base client ?

Some clients have to create their own search API.

[nabobalis]

I guess what I am trying to get at is that I think you need to take a step back and work out how errors propagate through the clients currently.

See if they do, how they do so, then work to see how you can extend that. Returning a string is not something that can or should be done in this case. You will need to work out the path for errors first.

Hopefully that helps.

[ryuusama09]

Let me start from the basics .
There are a number of clients right now ...

1. The generic client based clients use the same search method from their base class, which calls the scraper to search the data. The task here would be to make the errors return from the scraper in the function call recursion .

2. These are the ones that use overriden search methods ( from the base client) . They are : JSOC , VSO , HEK , CDAWEB , HEC.

for example CDAWEB uses _get_remote_files without calling any client and directly uses get method. Here the error from get request can easily propagated back . Other clients will have similar methodology ( I will test it out).

As for the warning string , like I mentioned earlier , we can add a warning attribute to the unifiedResponse object to show the warning message for 0 results due to server error .
Using the dataretriever clients , I wanted to test out the skeletal changes that are to made like in the client , the response object and how the system is performing as generic client based clients are easy and modular to use .
I hope I am correct

[nabobalis]

Let me start from the basics . There are a number of clients right now ...

1. The generic client based clients use the same search method from their base class, which calls the scraper to search the data. The task here would be to make the errors return from the scraper in the function call recursion .

But what errors need propagating from the scraper and how will that be done?

2. These are the ones that use overriden search methods ( from the base client) . They are : JSOC , VSO , HEK , CDAWEB , HEC.

for example CDAWEB uses _get_remote_files without calling any client and directly uses get method. Here the error from get request can easily propagated back . Other clients will have similar methodology ( I will test it out).

How do these clients handle errors? Are they raised? Do they break the search command?

As for the warning string , like I mentioned earlier , we can add a warning attribute to the unifiedResponse object to show the warning message for 0 results due to server error . Using the dataretriever clients , I wanted to test out the skeletal changes that are to made like in the client , the response object and how the system is performing as generic client based clients are easy and modular to use . I hope I am correct

Adding a ".errors" to the return from search mirrors how we do it for results and I think I am in favour of it on the surface. But it needs to be more than just a string. If you check the errors returned by Fido.fetch for example.

[ryuusama09]

But what errors need propagating from the scraper and how will that be done?

after x number of retries , if we still get an error from a particular link , we return the http_err object to signify that even after attempting so many times , we are getting an error signifying a server issue .

How do these clients handle errors? Are they raised? Do they break the search command?

Like I mentioned , Ill have to thoroughly test out the non generic clients . Right now , I can use generic clients to design the changes in the unifiedresponse object and observe the behaviour in different scenarios.

Adding a ".errors" to the return from search mirrors how we do it for results and I think I am in favour of it on the surface. But it needs to be more than just a string. If you check the errors returned by Fido.fetch for example.

Do you suggest anything particularly ? Like making a new error type for 0 response server error ? . Is it really required . Moreover aren't we just returning a waning to the user instead of throwing an error . shouldn't error 504 do the work as it means server didnt response resulting in a timeout ?

[nabobalis]

But what errors need propagating from the scraper and how will that be done?

after x number of retries , if we still get an error from a particular link , we return the http_err object to signify that even after attempting so many times , we are getting an error signifying a server issue .

Is that the only error?

How do these clients handle errors? Are they raised? Do they break the search command?

Like I mentioned , Ill have to thoroughly test out the non generic clients . Right now , I can use generic clients to design the changes in the unifiedresponse object and observe the behaviour in different scenarios.

Until we know how they handle errors (and we should document this in the developer docs for Fido), I don't think we continue forward.

Adding a ".errors" to the return from search mirrors how we do it for results and I think I am in favour of it on the surface. But it needs to be more than just a string. If you check the errors returned by Fido.fetch for example.

Do you suggest anything particularly ? Like making a new error type for 0 response server error ? . Is it really required . Moreover aren't we just returning a waning to the user instead of throwing an error . shouldn't error 504 do the work as it means server didnt response resulting in a timeout ?

There are several things in play.

1. How we report server fails to users.
2. How Clients should handle servers errors, do they raise a warning or do they raise an error?
3. What does Fido do when a client errors due to a failed search?

These are the questions we have to answer before we can start to code a solution.

[ryuusama09]

Is that the only error?

the http_err object is the standard interface , which can throw all the other http errors . So yes , I think that is the only error being thrown by the scraper currently.

How we report server fails to users.
How Clients should handle servers errors, do they raise a warning or do they raise an error?

This is a collective decision , whether we should raise an error or raise a warning.

What does Fido do when a client errors due to a failed search?

What should it do except for raising error/warning ? . The best we can do is print a detailed error log regarding which server went down with other specifics .
I don't think this issue is good first anymore😅

[nabobalis]

Is that the only error?

the http_err object is the standard interface , which can throw all the other http errors . So yes , I think that is the only error being thrown by the scraper currently.

And this covers what http errors?

How we report server fails to users.
How Clients should handle servers errors, do they raise a warning or do they raise an error?

This is a collective decision , whether we should raise an error or raise a warning.

What does Fido do when a client errors due to a failed search?

What should it do except for raising error/warning ? .

What does it do currently?

The best we can do is print a detailed error log regarding which server went down with other specifics . I don't think this issue is good first anymore😅

Yeah this probably needs more questions to be answered with some design work before we under take this.

[ryuusama09]

And this covers what http errors?

currently just 404 and 429

What does it do currently?

I am not sure , but if i am not wrong , it doesnt throw any error / warning (except for the pre query errors like date-range validation). It just returns empty response to the user .

[nabobalis]

And this covers what http errors?

currently just 404 and 429

Ok, are those all the one we need to worry about?
Those are raised by the scraper or just ignored?

What does it do currently?

I am not sure , but if i am not wrong , it doesnt throw any error / warning (except for the pre query errors like date-range validation). It just returns empty response to the user .

Without knowing, we can't progress on this.

It might be better to try another issue. Sorry about this.

[ryuusama09]

Ok, are those all the one we need to worry about?
Those are raised by the scraper or just ignored?

Nope , I think the scraper can be more robust , like handling different types of errors like 504 , and 404 (which is currently being ignored) and handling several edge cases. Can I raise a feature request to make the scraper more robust and work on it ? . So that later on it supports all the changes that might be done in this issue.

[nabobalis]

Nope , I think the scraper can be more robust , like handling different types of errors like 504 , and 404 (which is currently being ignored) and handling several edge cases. Can I raise a feature request to make the scraper more robust and work on it ? . So that later on it supports all the changes that might be done in this issue.

Sure.

[nabobalis]

As things stand, I don't think this PR will be merged in.

For now the original issue was with the scraper, so we should seperate them in any follow up PRs.

Thanks for your understanding.