supabase / pg_net

A PostgreSQL extension that enables asynchronous (non-blocking) HTTP/HTTPS requests with SQL
https://supabase.github.io/pg_net
Apache License 2.0
224 stars 18 forks source link

Test Failures due to Timeout #94

Closed TheOtherBrian1 closed 1 year ago

TheOtherBrian1 commented 1 year ago

Bug report

Describe the bug

I noticed that my tests unexpectedly failed despite only making changes to the documentation. The root cause appears to be timeout errors resulting from the httpbin endpoints being excessively slow (over 2 seconds for response). These timeout errors are not accounted for in the tests, causing them to fail even with functional code.

To Reproduce

To replicate, simply run the tests. A failure typically occurs due to the unanticipated timeout error.

Expected behavior

The tests should retry requests if there is a timeout error.

steve-chavez commented 1 year ago

The root cause appears to be timeout errors resulting from the httpbin endpoints being excessively slow (over 2 seconds for response).

@TheOtherBrian1 True, just noticed that as well. I think we should change those tests to use our Nginx server(one is started for the test suite) endpoints or Postman endpoints.

TheOtherBrian1 commented 1 year ago

@steve-chavez I could modify the tests to work with the Postman Echo API. It's designed specifically for this kind of testing, and can be easily swapped with httpbin. Looking through the tests, it would probably only take me half an hour to an hour to implement. Timeouts are still bound to happen occasionally, but it would be an improvement.

steve-chavez commented 1 year ago

@TheOtherBrian1 Cool. Please go ahead!

TheOtherBrian1 commented 1 year ago

@steve-chavez I added my changes to a pull request, but the automatic tests were taking an unusually long amount of time (more than an hour). I decided to close my request to prevent the action from maxing out.

The changes I made were fairly mild, so I'm hesitant to think that I accidentally created an infinite loop that held up the server. It's possible that because the actions are dependent on the tests, the modifications interfered with their normal function.

By default Github actions will timeout after 6 hours, which seems excessive in this context. It is probably wise to lower the timeout limit.

TheOtherBrian1 commented 1 year ago

@steve-chavez I have concerns regarding the test_http_requests_deleted_after_ttl test function. It expects entries in the net._http_response table to be deleted after 5 seconds of being inserted. However, upon reviewing the worker.c file, specifically lines 236 and 39, it seems that the deletion cycle has been adjusted to occur every 6 hours.

I may be misinterpreting the C code, but if my understanding is accurate, this test does not seem feasible and may need to be removed or commented out. You have a lot more insight on the inner workings of the extension than I do, so I would like to know your interpretation of this problem.