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

Use exception in OCI8 driver, instead of relying on assert #6596

Open
wants to merge 1 commit into
base: 4.2.x
Choose a base branch
from

Conversation

segrax
Copy link

@segrax segrax commented Nov 13, 2024

Fixes a bug where disabled asserts in production left oci_parse() errors unchecked. Now uses explicit checks and exceptions

Fixes #6595

Q A
Type bug
Fixed issues #6595

Summary

Fixes a bug where disabled asserts in production left oci_parse()
errors unchecked. Now uses explicit checks and exceptions

@segrax segrax changed the title Replace assert with exception in OCI8 driver Use exception in OCI8 driver, instead of relying on assert Nov 13, 2024
Copy link
Member

@derrabus derrabus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. Can we cover this change with a test?

src/Driver/OCI8/Connection.php Outdated Show resolved Hide resolved
@segrax
Copy link
Author

segrax commented Nov 14, 2024

Thank you. Can we cover this change with a test?

yeah, test has just been added

Fixes a bug where disabled asserts in production left oci_parse()
errors unchecked. Now uses explicit checks and exceptions

Add test for exception on oci_parse failure

Fixes doctrine#6595
@segrax segrax force-pushed the 6595-oci8-replace-asserts-with-exceptions branch from 80c3832 to 0e085ba Compare November 14, 2024 08:01
@segrax
Copy link
Author

segrax commented Nov 14, 2024

@derrabus should be all good, the last failing test seems to be unrelated (and has happened a couple of times now)

Failures
 - sqlite (exited 1) - sqlite not installed. An error occurred during installation:
 The remote server returned an error: (503) Server Unavailable. Service Unavailable: Back-end server is at capacity
Sorry, we tried running command for 3 times and all attempts were unsuccessful!

Get-ChildItem : Cannot find path 'C:\tools\php' because it does not exist.
At line:6 char:3
+   Get-ChildItem -Path c:\tools\php
+   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : ObjectNotFound: (C:\tools\php:String) [Get-ChildItem], ItemNotFoundException
    + FullyQualifiedErrorId : PathNotFound,Microsoft.PowerShell.Commands.GetChildItemCommand
 
cd : Cannot find path 'C:\tools\php' because it does not exist

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

Successfully merging this pull request may close these issues.

OCI8 driver does not check result from oci_parse, relies on disabled assert in production
2 participants