Skip to content

[ExpressionLanguage][Node][BinaryNode] Process division by zero #34814

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

[ExpressionLanguage][Node][BinaryNode] Process division by zero #34814

wants to merge 1 commit into from

Conversation

tigr1991
Copy link
Contributor

@tigr1991 tigr1991 commented Dec 4, 2019

Q A
Branch? master
Bug fix? no
New feature? yes
Deprecations? no
Tickets
License MIT
Doc PR

To be able to catch the error in expression like 10 / 0
Before PR:

try {
    1 / 0;
} catch (\Throwable $e) {
    // It won't be caught
    // PHP Warning:  Division by zero in...
}

try {
    1 % 0;
} catch (\Throwable $e) {
    // It will be caught
    // \DivisionByZeroError with message `Modulo by zero`
}

After PR:

try {
    1 / 0;
} catch (\Throwable $e) {
    // It will be caught
    // \DivisionByZeroError with message `Division by zero`
}

@tigr1991 tigr1991 force-pushed the expression_language/exceptions branch from b648e75 to 6b55e89 Compare December 4, 2019 11:51
@tigr1991 tigr1991 force-pushed the expression_language/exceptions branch from 6b55e89 to 75056da Compare December 4, 2019 11:54
@nicolas-grekas nicolas-grekas added this to the next milestone Dec 4, 2019
@tigr1991 tigr1991 mentioned this pull request Dec 5, 2019
Closed
@xabbuh
Copy link
Member

xabbuh commented Dec 5, 2019

IMO we should treat this as a bugfix. Evaluating an expression shouldn't trigger a PHP warning.

@nicolas-grekas
Copy link
Member

Closing in favor of #34842: we merge lower branches into upper ones periodically so they'll get the fixes too (it's written in the PR template ;) )

@nicolas-grekas nicolas-grekas modified the milestones: next, 5.1 May 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
ExpressionLanguage Feature Status: Needs Review
Projects
None yet
Milestone
5.1
Development

Successfully merging this pull request may close these issues.
4 participants
@tigr1991 @xabbuh @nicolas-grekas @carsonbot