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

add logging to lockfile related Exceptions #1149

Merged
merged 2 commits into from
Feb 13, 2021
Merged

Conversation

seansan
Copy link
Contributor

@seansan seansan commented Aug 17, 2020

…ip issues)

Suggestion to add some debugging to lockfile (to identify fileownership issues)

Description (*)

Related Pull Requests

Fixed Issues (if relevant)

  1. Fixes OpenMage/magento-lts#<issue_number>

Manual testing scenarios (*)

  1. ...
  2. ...

Questions or comments

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All automated tests passed successfully (all builds are green)

…ip issues)

Suggestion to add some debugging to lockfile (to identify fileownership issues)
@github-actions github-actions bot added the Component: Index Relates to Mage_Index label Aug 17, 2020
@jfksk
Copy link

jfksk commented Aug 17, 2020

Shouldn't that catch "Error" instead of "Exception" ?
"Error" and "Exception" do implement "Throwable" but "Error" does not extend "Exception".

OT (as this is a bit of an hassle to log):
Maybe it would be nice to change
Mage::logException(Exception $e)
to
Mage::logException(Throwable $e)

Edit:
Okay, I think I see now what you did here. I'd rather make this a bit more explicit (not thru the error-handler).

@sreichel
Copy link
Contributor

sreichel commented Aug 22, 2020

@seansan I'd like to close this, or ...

Please add a description and steps to reproduce.

@Flyingmana
Copy link
Contributor

Reopened after talking with @seansan

Will also do a small modification later, so the exceptions get thrown again after the logging, so we do not cause a BC break.

@Flyingmana Flyingmana reopened this Aug 24, 2020
@Flyingmana Flyingmana merged commit 111adf0 into OpenMage:20.0 Feb 13, 2021
@github-actions
Copy link
Contributor

Unit Test Results

1 files  1 suites   0s ⏱️
0 tests 0 ✔️ 0 💤 0 ❌
2 runs  2 ✔️ 0 💤 0 ❌

Results for commit 111adf0.

@Flyingmana Flyingmana changed the title Suggestion to add some debugging to lockfile (to identify fileownersh… add logging to lockfile related Exceptions Feb 14, 2021
@sreichel sreichel added this to the Release 20.0.7 milestone Feb 16, 2021
sreichel pushed a commit that referenced this pull request Nov 19, 2022
#1149)

* Suggestion to add some debugging to lockfile (to identify fileownership issues)

Suggestion to add some debugging to lockfile (to identify fileownership issues)

* keep the exception to bubble up

Co-authored-by: Daniel Fahlke <flyingmana@googlemail.com>
@sreichel sreichel mentioned this pull request Nov 19, 2022
4 tasks
fballiano pushed a commit that referenced this pull request Nov 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants