requests.exceptions.HTTPError not handled by praw

[endlisnis]

Describe the Bug

When I was using submit_image, praw encountered a 500 internal server error.

I have a handler for praw.exceptions.RedditAPIException, but that did not get caught because a requests.exceptions.HTTPError was raised instead of a praw.exceptions.RedditAPIException.

I assume that the intent is to have praw except exceptions from "requests" and raise them as praw exceptions (to avoid clients depending on knowledge of internal design).

$ python3 -m pip freeze | grep -e requests -e praw
praw==7.5.0
prawcore==2.3.0
requests==2.26.0
requests-file==1.4.3
requests-oauthlib==1.3.0
requests-toolbelt==0.9.1
requests-unixsocket==0.2.0

Desired Result

praw should have raised a praw.exceptions.RedditAPIException (or one of the classes that inherit from that)

Relevant Logs

File "/home/rolf/weather/redditSend.py", line 160, in postImage                                                                                                                                                    
    submission = sub.submit_image(                                                                                                                                                                                   
  File "/home/rolf/.local/lib/python3.8/site-packages/praw/models/reddit/subreddit.py", line 1188, in submit_image                                                                                                   
    "title": title,                                                                                                                                                                                                  
  File "/home/rolf/.local/lib/python3.8/site-packages/praw/models/reddit/subreddit.py", line 701, in _upload_media                                                                                                   
    response.raise_for_status()                                                                                                                                                                                      
  File "/home/rolf/.local/lib/python3.8/site-packages/requests/models.py", line 953, in raise_for_status                                                                                                             
    raise HTTPError(http_error_msg, response=self)                                                                                                                                                                   
requests.exceptions.HTTPError: 500 Server Error: Internal Server Error for url: https://reddit-uploaded-media.s3-accelerate.amazonaws.com/

Code to reproduce the bug

# I don't have a reproduction because in requires reddit to send back a 500.

My code example does not include the Reddit() initialization to prevent credential leakage.

Yes

This code has previously worked as intended.

No

Operating System/Environment

Linux Mint 20.2 Cinnamon

Python Version

Python 3.8.10

PRAW Version

7.5.0

Prawcore Version

2.3.0

Anything else?

No response

[endlisnis]

Do I have a valid issue here? Should users catch exceptions raised by internal libraries used by praw or should praw catch and raise praw exceptions?

[LilSpazJoekp]

This probably should be caught by PRAW. But this feature drills down directly to the underlying network package (requests) and bypasses the error handler in place in prawcore.

Yes, either PRAW or prawcore should catch this like you suggest.

[bboe]

Yes, this exception should be caught. It's one of the few rare places where PRAW is directly making a request and we don't have special exception handling around it.

@endlisnis would you like to try to add this exception handling in?

[endlisnis]

Sure. I can give it a shot. I have a patch ready. Is there some sort of test suite I can run on my change?

[LilSpazJoekp]

Yes. Should be able to do just pytest and it should run the tests. You'll need to install the dev requirements with: pip install ".[dev]".

[endlisnis]

OK. I have a pull request ready. It passed pytest, and I fixed the lint problem; although I don't understand the rules for where to put imports; but I just used the suggestion.

[endlisnis]

I'm having a hard time getting a unit test to work. My first attempt was to just literally duplicate the "test_submit_image2" test, but even a literal duplicate of that test fails. I'm perplexed. I suspect it has something to do with WebsocketMock and use_cassette(), but I can't figure out what those things mean.

Here's my test:

    @mock.patch("time.sleep", return_value=None)
    @mock.patch(
        "websocket.create_connection",
        return_value=WebsocketMock(
            "k5rhpg", "k5rhsu", "k5rhx3"  # update with cassette
        ),
    )
    def test_submit_image2(self, _, __):
        self.reddit.read_only = False
        with self.use_cassette():
            subreddit = self.reddit.subreddit(pytest.placeholders.test_subreddit)
            for i, file_name in enumerate(("test.png", "test.jpg", "test.gif")):
                image = self.image_path(file_name)

                submission = subreddit.submit_image(f"Test Title {i}", image)
                assert submission.author == self.reddit.config.username
                assert submission.is_reddit_media_domain
                assert submission.title == f"Test Title {i}"

And here's the failure output:

E           prawcore.exceptions.ResponseException: received 401 HTTP response

../../.local/lib/python3.8/site-packages/prawcore/auth.py:38: ResponseException

[LilSpazJoekp]

You'll want to do something similar to this test:

praw/tests/unit/models/reddit/test_subreddit.py

Lines 58 to 71 in b009cbf

	@mock.patch(
	"praw.Reddit.post", return_value={"json": {"data": {"websocket_url": ""}}}
	)
	@mock.patch(
	"praw.models.Subreddit._upload_media",
	return_value=("fake_media_url", "fake_websocket_url"),
	)
	@mock.patch("websocket.create_connection")
	def test_invalid_media(self, connection_mock, _mock_upload_media, _mock_post):
	connection_mock().recv.return_value = json.dumps(
	{"payload": {}, "type": "failed"}
	)
	with pytest.raises(MediaPostFailed):
	self.reddit.subreddit("test").submit_image("Test", "dummy path")

[endlisnis]

Thanks for the pointer. I was able to get something working, but I have to say that writing the test was about 100 times harder than writing the actual code.
Anyway, I'm just waiting for someone to start the tests on my latest commit.

[github-actions]

This issue is stale because it has been open for 20 days with no activity. Remove the Stale label or comment or this will be closed in 10 days.

[endlisnis]

I'm still working on this. Just having some trouble getting the testing code working. Thanks for the reminder, bot.

[LilSpazJoekp]

Sounds good! Would you like some help? If so, feel free to open a PR and I can take a look.

[LilSpazJoekp]

Just checking in on if this is still something you're working on.

[endlisnis]

To be honest, I gave up on this. I got distracted by life stuff. I'm going on paternity leave starting in June for several months. It's possible that I might get a chance to look at it then, but I wouldn't count on it. Feel free to give this to someone else if you want.

[LilSpazJoekp]

No worries! I'm on parental leave too so my time is limited as well. I'll take care of this for ya!