RecursionError with passthru and _real_send override
Seluj78 opened this issue · 33 comments
Describe the bug
When using the responses._real_send = self.r_mock.unbound_on_send() solution described here, I am getting a recursion error.
Additional context
This issue replaces this one
Version of responses
0.23.3
Steps to Reproduce
You can run unittest on this file and get the error :)
import os
import re
import unittest
import requests
os.environ["ENVIRONMENT"] = "tests"
import responses
def my_function():
# Send an email
requests.post("https://example.org")
# Do something else with the passthru
requests.post("http://localhost:7700/indexes/test/documents")
return "OK"
class _TestCase(unittest.TestCase):
def setUp(self):
self.r_mock = responses.RequestsMock(assert_all_requests_are_fired=True)
self.r_mock.start()
self.r_mock.add_passthru(re.compile(rf"http://localhost:7700.*"))
responses._real_send = self.r_mock.unbound_on_send()
def tearDown(self):
self.r_mock.stop()
self.r_mock.reset()
class MyTest(_TestCase):
def test_indexing(self):
self.r_mock.add(responses.POST, "https://example.org", status=200)
self.assertEqual("OK", my_function())Expected Result
The test should run fine
Actual Result
============================= test session starts ==============================
collecting ... collected 1 item
reproduce.py::MyTest::test_indexing
============================== 1 failed in 0.19s ===============================
FAILED [100%]
reproduce.py:34 (MyTest.test_indexing)
self = <reproduce.MyTest testMethod=test_indexing>
def test_indexing(self):
self.r_mock.add(responses.POST, "https://example.org", status=200)
> self.assertEqual("OK", my_function())
reproduce.py:37:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
reproduce.py:16: in my_function
requests.post("http://localhost:7700/indexes/test/documents")
.venv/lib/python3.11/site-packages/requests/api.py:115: in post
return request("post", url, data=data, json=json, **kwargs)
.venv/lib/python3.11/site-packages/requests/api.py:59: in request
return session.request(method=method, url=url, **kwargs)
.venv/lib/python3.11/site-packages/requests/sessions.py:587: in request
resp = self.send(prep, **send_kwargs)
.venv/lib/python3.11/site-packages/requests/sessions.py:701: in send
r = adapter.send(request, **kwargs)
.venv/lib/python3.11/site-packages/responses/__init__.py:1127: in send
return self._on_request(adapter, request, **kwargs)
.venv/lib/python3.11/site-packages/responses/__init__.py:1033: in _on_request
return _real_send(adapter, request, **kwargs)
.venv/lib/python3.11/site-packages/responses/__init__.py:1127: in send
return self._on_request(adapter, request, **kwargs)
E RecursionError: maximum recursion depth exceeded while calling a Python object
!!! Recursion detected (same locals & position)
is it possible to simplify an example to avoid any application deployment?
Sure enough, I just updated the issue. I had it that way to make sure it was similar to my real setup but it woks that way as well.
@Seluj78 and we certainly need flask here ?
and 4 test cases ?
I've put 4 test cases to showcase that 3 work, one doesn't, and it seems like it's the one with the passthru that doesn't work. I don't know about flask, let me see if I can reduce the example even further.
Updated once more without flask :) Sorry about that
@Seluj78 looking at example I think the issue is that you mock the request library with responses. Then within responses you add passthrough which leads for responses to call _real_send. While in this case this function is overwritten by again mocked responses object.
this is different from the solution I provided in moto since in moto there are 2 responses objects.
in your case you will need to intercept it with another responses, eg
import os
import re
import unittest
import requests
os.environ["ENVIRONMENT"] = "tests"
import responses
def my_function():
# Send an email
requests.post("https://example.org")
# Do something else with the passthru
requests.post("http://localhost:7700/indexes/test/documents")
return "OK"
class _TestCase(unittest.TestCase):
def setUp(self):
self.r_mock = responses.RequestsMock(assert_all_requests_are_fired=True)
self.r_mock2 = responses.RequestsMock(assert_all_requests_are_fired=True)
self.r_mock2.start()
self.r_mock.start()
self.r_mock.add_passthru(re.compile(rf"http://localhost:7700.*"))
responses._real_send = self.r_mock2.unbound_on_send()
def tearDown(self):
self.r_mock.stop()
self.r_mock2.stop()
self.r_mock.reset()
self.r_mock2.reset()
class MyTest(_TestCase):
def test_indexing(self):
self.r_mock.add(responses.POST, "https://example.org", status=200)
self.r_mock2.add(responses.POST, re.compile(rf"http://localhost:7700.*"), status=200)
self.assertEqual("OK", my_function())if you want to send the real response (eg you run e2e test with real application deployed), then you do not need to overwrite _real_send at all. Just leave it as is
I see. The problem then is that, without overwriting _real_send, I still get a problem with my class setup of responses and how different registeries are used combined with moto, see #669
I didn't find a good example in your documentation about how to setup responses like I did here without having the problem I mentioned in the other issue
@Seluj78 do you try to add pass through in your version of responses, not in moto ?
Yes that's correct, I never touch moto's version of responses, the only thing I do (in the setUp method of my base testing class) is
self.r_mock = responses.RequestsMock(assert_all_requests_are_fired=True)
self.r_mock.start()
self.r_mock.add_passthru(re.compile(rf"{MEILISEARCH_URL}.*"))
and then try to self.r_mock.add...
ok, then we need to find a way to tell moto that it must real send into mocked version, while your call to responses should result in passing through into real requests.
that means we need to apply responses._real_send = self.r_mock2.unbound_on_send() not on the lib level.
@bblommers any though on this ?
I can think of changing responses to add this as argument, however, would be better if we can articulate from outside without touching codebase
Thank you for your time 🙏 Do let me know if you need any further information.
Hi @beliaev-maksim!
Do you mean something like responses._real_send = responses_mock_created_by_moto.unbound_on_send(), for that to be part of Moto? Or am I misunderstanding you?
@Seluj78 @bblommers
please look at #671
I updated responses in the way that it will allow to explicitly set real send from custom adapter. (Please see test case in PR)
that means if in moto there will be a way for users to pass as argument the adapter send function, then users will be able to instantiate responses, then pass unbound on send method to moto and enjoy both libraries.
@bblommers feasible change on moto side ?
I would like to test the change on feature branches on both repos before we merge it anywhere, then if positive. We first release new responses, then moto can catch up
Wonderful! What would you need from me ? A comprehensive test case that includes moto and responses for you to test with ?
Wonderful! What would you need from me ? A comprehensive test case that includes moto and responses for you to test with ?
That would be very useful @Seluj78 - once I have a test case, I will have a look how to best integrate this in Moto.
Alright. Give me a few, I'll try and modify the test case I have provided here to include moto and reproduce the error(s) I was having with the double responses registry !
Here is a test case I made that showcases the two errors. I hope you'll find it useful @bblommers @beliaev-maksim !
import re
import unittest
import requests
import responses
from moto import mock_ec2
from moto.core import patch_resource
import boto3
ec2_resource = boto3.resource("ec2", region_name="us-east-1")
def my_function():
# Send an email
requests.post("https://example.org")
# Do something else with the passthru
requests.post("http://localhost:7700/indexes/test/documents")
return "OK"
def create_instance_and_send_email():
ec2_resource.create_instances(
ImageId="ami-12345678",
MinCount=1,
MaxCount=1,
InstanceType="t2.micro",
KeyName="my-key-pair",
)
my_function()
return "OK"
class _TestCase(unittest.TestCase):
def setUp(self):
self.r_mock = responses.RequestsMock(assert_all_requests_are_fired=True)
self.r_mock.start()
self.r_mock.add_passthru(re.compile(rf"http://localhost:7700.*"))
# If you leave the _real_send commented, you will get an error because
# self.r_mock.add(responses.POST, "https://example.org", status=200)
# will not have been executed. You can verify this by putting a breakpoint inside
# `create_instance_and_send_email` and running it in debug mode (I'm using Pycharm to do this).
# Once at the breakpoint, you can do `import responses; responses.registered()` and see that nothing is registered.
# If you're able with your debugger, move back up the stacktrace back to the test `test_create_instance`
# and run once more `import responses; responses.registered()` as well as `self.r_mock.registered()`
# to see the different and what I think the problem is.
# If you uncomment _real_send4, you will get the RecursionError previously mentioned.
# responses._real_send = self.r_mock.unbound_on_send()
def tearDown(self):
self.r_mock.stop()
self.r_mock.reset()
# I have not tried to use @mock_xxx on the base class, I don't know if it will work.
@mock_ec2
class MyTest(_TestCase):
def setUp(self):
super().setUp()
patch_resource(ec2_resource)
def test_indexing(self):
self.r_mock.add(responses.POST, "https://example.org", status=200)
self.assertEqual("OK", my_function())
def test_create_instance(self):
self.r_mock.add(responses.POST, "https://example.org", status=200)
self.assertEqual("OK", create_instance_and_send_email())Do let me know if you have questions about it. 😄
Thanks @Seluj78, that makes sense.
I've managed to get this to work without any modifications to Moto, by installing the PR from @beliaev-maksim .
pip install https://github.com/beliaev-maksim/responses/archive/refs/heads/mbeliaev/real_send.zip
The only change I made was in _TestCase.setup:
from moto.core.models import responses_mock
def setUp(self):
self.r_mock = responses.RequestsMock(assert_all_requests_are_fired=True)
self.r_mock.start()
self.r_mock.add_passthru(re.compile(rf"http://localhost:7700.*"))
responses_mock._real_send = self.r_mock.unbound_on_send()
@beliaev-maksim The RequestsMock that Moto uses is initialized on import, so changing that to pass the real_adapter_send-argument is not trivial. (Do-able, I guess, just not easy.)
Is it an option to make the attribute part of the public API instead? I.e. RequestsMock.real_send = ..?
Being able to set this on demand, instead of needing to know the value on instantiation, would be much easier (for me at least.. 🙂 )
@bblommers not encouraged, but you can try RequestsMock._real_send
@beliaev-maksim Or alternatively - can there be a RequestsMock.set_real_send(..)? Again, not a must, but it would make things much easier.
@bblommers can you please try so far with private attribute to see if that works?
We can consider expanding it in the future
@beliaev-maksim Yes, it does work! @Seluj78 's test case works when overwriting _real_send - sorry if that wasn't clear.
(I still haven't had the time to test your PR @beliaev-maksim, but I will let you know when I do)
I'm having trouble making the test setup work. @bblommers could you send me what you had that worked ?
@Seluj78 Slightly adapted from your own test:
import unittest
import requests
import responses
from moto import mock_dynamodb
from moto.core.models import responses_mock
import boto3
def my_function():
# Mock this website
requests.post("https://example.org")
# Passthrough this website
assert requests.get("http://ip.jsontest.com").status_code == 200
return "OK"
def create_instance_and_send_email():
ddb = boto3.client("dynamodb", "us-east-1")
assert ddb.list_tables()["TableNames"] == []
my_function()
return "OK"
@mock_dynamodb
class MyTest(unittest.TestCase):
def setUp(self):
self.r_mock = responses.RequestsMock(assert_all_requests_are_fired=True)
responses_mock._real_send = self.r_mock.unbound_on_send()
self.r_mock.start()
self.r_mock.add_passthru("http://ip.jsontest.com")
def tearDown(self):
self.r_mock.stop()
self.r_mock.reset()
def test_indexing(self):
self.r_mock.add(responses.POST, "https://example.org", status=200)
self.assertEqual("OK", my_function())
def test_create_instance(self):
self.r_mock.add(responses.POST, "https://example.org", status=200)
self.assertEqual("OK", create_instance_and_send_email())
This fails with responses==0.23.3, but succeeds with the latest: pip install https://github.com/getsentry/responses/archive/refs/heads/master.zip
@bblommers these fail for me with
FAILED [100%]
reproduce.py:37 (MyTest.test_indexing)
self = <reproduce.MyTest testMethod=test_indexing>
def tearDown(self):
> self.r_mock.stop()
reproduce.py:35:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
self = <responses.RequestsMock object at 0x103f55450>, allow_assert = True
def stop(self, allow_assert: bool = True) -> None:
if self._patcher:
# prevent stopping unstarted patchers
self._patcher.stop()
# once patcher is stopped, clean it. This is required to create a new
# fresh patcher on self.start()
self._patcher = None
if not self.assert_all_requests_are_fired:
return
if not allow_assert:
return
not_called = [m for m in self.registered() if m.call_count == 0]
if not_called:
> raise AssertionError(
"Not all requests have been executed {0!r}".format(
[(match.method, match.url) for match in not_called]
)
)
E AssertionError: Not all requests have been executed [('POST', 'https://example.org/')]
.venv/lib/python3.11/site-packages/responses/__init__.py:1158: AssertionErrorUsing responses installed from master
@Seluj78 I had to explicitly uninstall responses first.
I've created an example repo that shows the test passes.
- the test file: https://github.com/bblommers/responses-passthru-example/blob/main/test/test.py
- the build process: https://github.com/bblommers/responses-passthru-example/blob/main/.github/workflows/test.yml
- the passing test: https://github.com/bblommers/responses-passthru-example/actions/runs/6422677597/job/17439670560
Ok thanks !
I'll be OOO for the next week and a half, I'll try it when I come back !
@beliaev-maksim Is there a way that the fix could make its way in a release on pypi ?
@Seluj78 can you confirm that it is working on master ?
@beliaev-maksim I just tried using @bblommers's way of installing and it worked on my reproduce test. I am currently running it on my 700 tests but it seems that it's all working ! I need to fix some tests (I'm getting some AssertionError: Not all requests have been executed) but other than that it works !! :D
It's working :)