Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added checker for _convertPath if the path is file #500

Merged
merged 5 commits into from Oct 29, 2017
Merged

Added checker for _convertPath if the path is file #500

merged 5 commits into from Oct 29, 2017

Conversation

ghost
Copy link

@ghost ghost commented Sep 27, 2017

Hello!

I want exclude from stacktrace a list files, example

$client = new Raven_Client($dsn, array(
    'excluded_app_paths' => ['vendor', 'tests', 'src/ExcludedFile.php'],
));

But if missing check on file, _convertPath added a slash in the end.

@stayallive
Copy link
Collaborator

stayallive commented Oct 1, 2017

Hey @personnage,

Good catch, I am thinking if we could get away with checking if the path ends with .php to detect if it's a file since this could add some overhead of statting files each time this check is ran. What do you think?

@ghost
Copy link
Author

ghost commented Oct 9, 2017

@stayallive, I think you're right.

@stayallive
Copy link
Collaborator

@personnage there is one more thing I noticed, this is also used to strip the prefix from an path in the stacktrace. This could mean a trace going through your excluded file leaves the filename empty.

We might need to filter out files ending in .php before they are being sent through the _convertPath and leave that function as it was. What about:

public function setExcludedAppPaths($value)
{
    if ($value) {
        $excluded_app_paths = array();

        foreach ((array)$value as $path) {
            $excluded_app_paths[] = substr($path, -4) !== '.php' ? $this->_convertPath($path) : $path;
        }
    } else {
        $excluded_app_paths = null;
    }

    $this->excluded_app_paths = $excluded_app_paths;

    return $this;
}

This way the other usages of _convertPath are not impacted.

(why no closure: It is not possible to use $this from anonymous function before PHP 5.4.0, savages)

@ghost
Copy link
Author

ghost commented Oct 16, 2017

@stayallive, done

@stayallive
Copy link
Collaborator

@personnage I added some tests to prove this is working.

@Jean85 / @ste93cry care to take a look at this? This might also be a thing to implement in 2.0 if not already.

@stayallive stayallive requested review from Jean85 and ste93cry October 21, 2017 14:54
@ghost
Copy link
Author

ghost commented Oct 29, 2017

personnage I added some tests to prove this is working.

@stayallive And I thank you for that.

Jean85 / ste93cry care to take a look at this?

Then I look forward to it.

@stayallive stayallive merged commit 11a2429 into getsentry:master Oct 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants