Skip to content
This repository has been archived by the owner on Sep 4, 2021. It is now read-only.

Do not assume http_response_header set during exception #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

skoranda
Copy link

The variable $http_response_header may not be set during an exception. A
typical use case is calling out to the Twitter API during Twitter
authentication when the callback URI has not been registered with
Twitter. Because of how the Twitter API responds $http_response_header
is not set and during the handling of the exception the missing
$http_response_header masks the real issue. This commit changes the code
to not assume that $http_response_header is set.

The variable $http_response_header may not be set during an exception. A
typical use case is calling out to the Twitter API during Twitter
authentication when the callback URI has not been registered with
Twitter. Because of how the Twitter API responds $http_response_header
is not set and during the handling of the exception the missing
$http_response_header masks the real issue. This commit changes the code
to not assume that $http_response_header is set.
@codecov
Copy link

codecov bot commented Apr 14, 2020

Codecov Report

Merging #3 into master will not change coverage.
The diff coverage is 0.00%.

@@           Coverage Diff            @@
##             master      #3   +/-   ##
========================================
  Coverage      0.00%   0.00%           
- Complexity       79      80    +1     
========================================
  Files             4       4           
  Lines           296     297    +1     
========================================
- Misses          296     297    +1     

@tvdijen
Copy link
Member

tvdijen commented Apr 14, 2020

Thanks @skoranda !

Not to self: if you merge this, make sure to backport and tag v0.9.2

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants