[2.0] Merge master differences in 2.x branch #526

Jean85 · 2017-12-22T17:34:47Z

This PR is intended to port all differences from 1.x to 2.x, so we do not regress on bugs and stuff in the new release. This is a recap of what I've covered so far, with all PR that we merged in master and not on 2.x:

Last master commit merged into 2.0: a997764

Exclude anonymous frames from app path #482: Exclude anonymous frames from app path (already taken care of in tests:

sentry-php/tests/StacktraceTest.php

Line 231 in 873ab3c

public function testFromBacktraceWithAnonymousFrame()

)

Fix bug that prevented to filter sensitive data when there is an exception with multiple 'values' #483: multiple sanitize (already taken care of in tests:

sentry-php/tests/Processor/SanitizeDataProcessorTest.php

Lines 47 to 51 in 873ab3c

    
           $this->assertEquals(SanitizeDataProcessor::STRING_MASK, $vars['password']); 
        
           $this->assertEquals(SanitizeDataProcessor::STRING_MASK, $vars['the_secret']); 
        
           $this->assertEquals(SanitizeDataProcessor::STRING_MASK, $vars['a_password_here']); 
        
           $this->assertEquals(SanitizeDataProcessor::STRING_MASK, $vars['mypasswd']); 
        
           $this->assertEquals(SanitizeDataProcessor::STRING_MASK, $vars['authorization']);

)

Add PHP 7.2 in CI #489: ported with 38be743
Prevent fatal error in Raven_Stacktrace #493: non existent file for stacktrace (already taken care of:

sentry-php/lib/Raven/Stacktrace.php

Line 227 in 873ab3c

if (!is_file($path) || !is_readable($path)) {

)
Added checker for _convertPath if the path is file #500: allow single files in excluded_app_paths (ported with aad776d)
Remove ini_set call for unneeded functionality #501: no longer applicable (Autoloader is done by composer)

Prevent segmentation fault in Reflection->getParameters() #504: segfault in stacktrace with missing function (already taken care of with generic catch:

sentry-php/lib/Raven/Stacktrace.php

Lines 367 to 372 in 873ab3c

    
               } else { 
        
                   $reflection = new \ReflectionFunction($frame['function']); 
        
               } 
        
           } catch (\ReflectionException $ex) { 
        
               return self::getFrameArgumentsValues($frame, $maxValueLength); 
        
           }

)

Use namespaced TestCase #506: upgrade to PHPUnit 6 (already done at base)

Changes for 1.9.x dev #509: remove suggestion of 5.2 port (already done:

sentry-php/composer.json

Lines 35 to 37 in 873ab3c

    
           "suggest": { 
        
               "monolog/monolog": "Automatically capture Monolog events as breadcrumbs" 
        
           },

)

Bugfix: Raven_CurlHandler -> join_timeout #510: curl related, no longer applicable
Do not capture E_ERROR type as fatal error #514: avoid double-capture with PHP 7.0 (done in 20c79ed)
Add setters for Raven Serializes #515: set serializers (done in d604f05)
Fix SENTRY_DSN handling with nullable values #522: handle nullable values for DSN (done in f844b5d)
Update changelog for version 1.8.2 #525: release 1.8.2
default value for array should be array #527: default values as array for frames (porting only the regression test, done in 6c7893c)
Adding breadcrumbs to sanitize / serialize. #538: serialize breadcrumbs too
Update changelog for version 1.8.3 #541: 1.8.3 release

nokitakaze · 2018-01-02T08:10:05Z

.travis.yml

@@ -4,11 +4,9 @@ php:
  - 5.6
  - 7.0
  - 7.1
-  - nightly


You should not delete nightly version. It is not 7.2, it is current dev version, so now it is 7.3. We always should know how our code running on new version of PHP

I disagree. 7.3 is now very far from being usable, and Travis will tag it while in beta stage, so we can add it at that point. (which will be in at least 8-9 months in the future from now).

Jean85 · 2018-01-02T14:27:31Z

Done! No longer WIP, please review!

ste93cry · 2018-01-02T14:43:26Z

.scrutinizer.yml

@@ -5,7 +5,7 @@ tools:
    php_code_coverage: true
    external_code_coverage:
        timeout: 2400 # There can be another pull request in progress
-        runs: 3 # PHP 5.6 + PHP 7.0 + PHP 7.1
+        runs: 3 # PHP 5.6 + PHP 7.0 + PHP 7.1 + PHP 7.2


Shouldn't this be 4? According to the comment and the count of the option it was 3 before adding PHP 7.2

Yep, that's right!! Fixing it now

Okay :(

nokitakaze · 2018-01-02T16:18:59Z

docs/integrations/laravel.rst

@@ -239,14 +252,14 @@ In the following example, we'll use a middleware:
        public function handle($request, Closure $next)
        {
            if (app()->bound('sentry')) {
-                /** @var \Raven\Client $sentry */
+                /** @var \Raven_Client $sentry */


I think it is a mistake

Yep it is! Fixing it, thanks for spotting it!

Umh... Wait a minute... It's like that because I copy-pasted from master, but simply replacing that may not work... The docs need to be fully updated to how the 2.x client will work, so maybe we can postpone that to a later PR... Also, @stayallive knows that Laravel integration way better than me!

Will for sure take a look at this very soon!

@stayallive ?

Jean85 · 2018-01-19T10:22:10Z

I've resolved conficts. Can we merge this now? I (and @ste93cry too I believe) would like to resolve the docs issues for the 2.0 in a separate PR

stayallive

Looks good. Will look at the docs separately.

nokitakaze · 2018-01-19T10:33:03Z

Erm, maybe we should wait until tests are completed successfully?

Jean85 · 2018-01-19T12:06:02Z

I did a mistake during the merge, it should be green now!

(cherry picked from commit 0904031)

(cherry picked from commit 7a297d0)

Jean85 · 2018-02-07T23:02:01Z

I've merged more stuff from master in this PR, it's now up to date.

@nokitakaze can you approve this? I wouldn't like to have more conflicts to solve here...

ste93cry · 2018-02-07T23:05:13Z

@Jean85 what about leaving this PR open until 2.x is ready to be released in alpha so that we can track the differences between the this branch and master? I know it would be difficult to fix conflicts, but maybe it's better to do it one time for all

Jean85 · 2018-02-07T23:18:28Z

It's not too difficult, the pain could be that we would be having PRs targeting wrong stuff or missing regression tests that we already have in here.

I can easily reopen a new PR like this one to start tracking new differences and merge it later, but this is already big IMO.

Jean85 added this to the Release 2.0 milestone Dec 22, 2017

Jean85 self-assigned this Dec 22, 2017

nokitakaze previously requested changes Jan 2, 2018

View reviewed changes

stayallive changed the title ~~[WIP] Merge master differences in 2.x branch~~ [2.0 ][WIP] Merge master differences in 2.x branch Jan 2, 2018

stayallive changed the title ~~[2.0 ][WIP] Merge master differences in 2.x branch~~ [2.0] [WIP] Merge master differences in 2.x branch Jan 2, 2018

Jean85 added 3 commits January 2, 2018 13:05

Realign text documents

7c126ad

Add PHP 7.2 in CI (like in 5958fdf)

0bfea95

Allow single files in the excluded paths option (port of #500 onto 2.x)

aad776d

Jean85 force-pushed the merge-master branch from 38be743 to aad776d Compare January 2, 2018 13:16

Jean85 added 4 commits January 2, 2018 14:22

Do not capture fatal errors under PHP 7+ (port of #514 onto 2.x)

20c79ed

Add setters for Serializers (port of #515 onto 2.x)

d604f05

Handle nullable value for DSN (porting #522 to 2.x branch)

f844b5d

Fix CS

0537ffc

Jean85 changed the title ~~[2.0] [WIP] Merge master differences in 2.x branch~~ [2.0] Merge master differences in 2.x branch Jan 2, 2018

Jean85 requested review from dcramer, stayallive, ste93cry and nokitakaze January 2, 2018 14:27

ste93cry requested changes Jan 2, 2018

View reviewed changes

Fix scrutinizer coverage count setting

1fec891

nokitakaze suggested changes Jan 2, 2018

View reviewed changes

Merge branch '2.0' into merge-master for resolving conflicts

89bcd3a

stayallive approved these changes Jan 19, 2018

View reviewed changes

ste93cry approved these changes Jan 19, 2018

View reviewed changes

Fix bad merge (and tests)

7c35324

Jean85 and others added 4 commits February 7, 2018 23:21

Porting regression test from master from PR #527

6c7893c

Adding breadcrumbs to sanitize / serialize. (#538) (plus fix CS)

9bdcf52

(cherry picked from commit 0904031)

Update changelog for version 1.8.3

83cc60e

(cherry picked from commit 7a297d0)

Merge branch '2.0' into merge-master to resolve conflicts

5f2243b

nokitakaze approved these changes Feb 8, 2018

View reviewed changes

Jean85 merged commit 1d61430 into 2.0 Feb 8, 2018

Jean85 deleted the merge-master branch February 8, 2018 08:25

Jean85 mentioned this pull request Feb 8, 2018

[2.0] Fix unused variable #542

Merged

Jean85 mentioned this pull request May 3, 2018

[2.0] Merge up to 1.9 #592

Merged

27 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[2.0] Merge master differences in 2.x branch #526

[2.0] Merge master differences in 2.x branch #526

Jean85 commented Dec 22, 2017 •

edited

Loading

nokitakaze Jan 2, 2018

Jean85 Jan 2, 2018

Jean85 commented Jan 2, 2018

ste93cry Jan 2, 2018

Jean85 Jan 2, 2018

nokitakaze Jan 2, 2018

Jean85 Jan 2, 2018

Jean85 Jan 2, 2018

stayallive Jan 2, 2018

nokitakaze Jan 19, 2018

Jean85 commented Jan 19, 2018

stayallive left a comment

nokitakaze commented Jan 19, 2018

Jean85 commented Jan 19, 2018

Jean85 commented Feb 7, 2018

ste93cry commented Feb 7, 2018

Jean85 commented Feb 7, 2018

	$this->assertEquals(SanitizeDataProcessor::STRING_MASK, $vars['password']);
	$this->assertEquals(SanitizeDataProcessor::STRING_MASK, $vars['the_secret']);
	$this->assertEquals(SanitizeDataProcessor::STRING_MASK, $vars['a_password_here']);
	$this->assertEquals(SanitizeDataProcessor::STRING_MASK, $vars['mypasswd']);
	$this->assertEquals(SanitizeDataProcessor::STRING_MASK, $vars['authorization']);

	} else {
	$reflection = new \ReflectionFunction($frame['function']);
	}
	} catch (\ReflectionException $ex) {
	return self::getFrameArgumentsValues($frame, $maxValueLength);
	}

	"suggest": {
	"monolog/monolog": "Automatically capture Monolog events as breadcrumbs"
	},

@@ @@ -4,11 +4,9 @@ php: @@
                 - 5.6
                 - 7.0
                 - 7.1
-                - nightly

[2.0] Merge master differences in 2.x branch #526

[2.0] Merge master differences in 2.x branch #526

Conversation

Jean85 commented Dec 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jean85 commented Jan 2, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jean85 commented Jan 19, 2018

stayallive left a comment

Choose a reason for hiding this comment

nokitakaze commented Jan 19, 2018

Jean85 commented Jan 19, 2018

Jean85 commented Feb 7, 2018

ste93cry commented Feb 7, 2018

Jean85 commented Feb 7, 2018

Jean85 commented Dec 22, 2017 •

edited

Loading