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

BaseActiveRecord save method - php docs do not state possible throwing of exception #20235

Closed
mandano opened this issue Jul 26, 2024 · 4 comments
Labels
type:docs Documentation

Comments

@mandano
Copy link

mandano commented Jul 26, 2024

What steps will reproduce the problem?

  • go to vendor/yiisoft/yii2/db/BaseActiveRecord.php
  • check [at]throws attribute in php docs
  • check insert() method php docs in ActiveRecord.php,
  • check update() method php docs in BaseActiveRecord.php

What is the expected result?

  • expected to contain [at]throws StaleObjectException, Exception or
    Throwable as update(), insert() methods could throw these
  • in case save() should not throw an exception, catch needed for update(), insert() methods

What do you get instead?

  • no [at]throws statement stated
  • or missing catch statements in save()

Additional info

Q A
Yii version 2.0.49.4
PHP version
Operating system Linux

save() method - BaseActiveRecord
grafik

insert() method - ActiveRecord
grafik

update() method - BaseActiveRecord
grafik

@samdark samdark added the type:docs Documentation label Jul 26, 2024
@samdark
Copy link
Member

samdark commented Jul 26, 2024

Should be added to phpdoc, indeed. Do you have time for a pull request?

@mandano
Copy link
Author

mandano commented Jul 29, 2024

Should be added to phpdoc, indeed. Do you have time for a pull request?

hey! i have time for a pr.
Btw what do you think about catching the exceptions inside of save(). as otherwise each user of save() needs to create a try catch, which bloats up that user's abstraction layer unnecessarily?

@handcode
Copy link

for phpdoc see #20146

about catching the exceptions inside of save():
I think this should/can not be done, as exceptions thrown within save() can be (or are) used to handle use cases like #8392 or #20253

@samdark samdark closed this as completed Sep 17, 2024
@samdark samdark reopened this Sep 17, 2024
@mandano
Copy link
Author

mandano commented Sep 21, 2024

ok, thank you.

@mandano mandano closed this as completed Sep 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:docs Documentation
Projects
None yet
Development

No branches or pull requests

3 participants