Fix content(raw=>1) dies when there is no response #397
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug where calling content(raw=>1) on a WWW::Mechanize object would die when there is no response object, addressing issue #384.
- Added null check for response object before calling content() method
- Added test coverage to verify raw content handling when no response exists
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| lib/WWW/Mechanize.pm | Added safety check to prevent calling content() on undefined response object |
| t/content.t | Added test case to verify raw content returns undef when no response exists |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| $content = $res->content() if $res; | ||
| } | ||
| elsif ( delete $params{decoded_by_headers} ) { | ||
| $content = $self->response()->decoded_content( charset => 'none' ); |
There was a problem hiding this comment.
Similar to the raw content case, this line could also die if response() returns undef. The same null check should be applied here for consistency.
| $content = $self->response()->decoded_content( charset => 'none' ); | |
| my $res = $self->response(); | |
| $content = $res->decoded_content( charset => 'none' ) if $res; |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Copilot makes a good point.
It seems to me the whole problem follows when user requests for content before any remote http call has been made.
Is there any way to prevent that? Or do we even want to?
There was a problem hiding this comment.
Is there any way to prevent that? Or do we even want to?
I think this is a good start. We can protect the user from themselves without introducing new types of errors. Even with really innocent looking changes, often someone will appear after a subsequent release to let us know that our fix has broken their code. So, it's good to be conservative here. I think this particular case is worth fixing, though.
There was a problem hiding this comment.
Then we are done and ready for merging?
2 participants
#384