Skip to content

Use Throwable as type hint in the exception callback#35713

Merged
DeepDiver1975 merged 1 commit intomasterfrom
bugfix/use-throwable-not-exception-as-typehint
Jul 3, 2019
Merged

Use Throwable as type hint in the exception callback#35713
DeepDiver1975 merged 1 commit intomasterfrom
bugfix/use-throwable-not-exception-as-typehint

Conversation

@DeepDiver1975
Copy link
Copy Markdown
Member

@DeepDiver1975 DeepDiver1975 commented Jul 1, 2019

Description

Not only exceptions can be thrown at runtime but also Errors. The common interface is \Throwable

Motivation and Context

Handle Errors properly as well

How Has This Been Tested?

  • add a wrong type declaration in the code which is executed on the webdav endpoint
  • see the error page properly popping up

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

Open tasks:

  • Backport (if applicable set "backport-request" label and remove when the backport was done)

Copy link
Copy Markdown
Contributor

@patrickjahns patrickjahns left a comment

Choose a reason for hiding this comment

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

LGTM

@patrickjahns patrickjahns self-requested a review July 1, 2019 17:38
Comment thread apps/dav/lib/Connector/Sabre/FilesPlugin.php Outdated
Comment thread apps/dav/lib/Files/BrowserErrorPagePlugin.php Outdated
Comment thread apps/dav/lib/Files/BrowserErrorPagePlugin.php Outdated
@DeepDiver1975 DeepDiver1975 force-pushed the bugfix/use-throwable-not-exception-as-typehint branch from cfbb985 to 32b9ac6 Compare July 2, 2019 06:57
@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 2, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@27d12b0). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #35713   +/-   ##
=========================================
  Coverage          ?   65.77%           
  Complexity        ?    18777           
=========================================
  Files             ?     1222           
  Lines             ?    70872           
  Branches          ?     1289           
=========================================
  Hits              ?    46617           
  Misses            ?    23877           
  Partials          ?      378
Flag Coverage Δ Complexity Δ
#javascript 53.7% <ø> (?) 0 <ø> (?)
#phpunit 67.15% <100%> (?) 18777 <4> (?)
Impacted Files Coverage Δ Complexity Δ
apps/dav/lib/Files/BrowserErrorPagePlugin.php 90% <100%> (ø) 5 <2> (?)
apps/dav/lib/Connector/Sabre/FilesPlugin.php 87.07% <100%> (ø) 77 <2> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 27d12b0...32b9ac6. Read the comment docs.

@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 2, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@27d12b0). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #35713   +/-   ##
=========================================
  Coverage          ?   65.77%           
  Complexity        ?    18777           
=========================================
  Files             ?     1222           
  Lines             ?    70872           
  Branches          ?     1289           
=========================================
  Hits              ?    46617           
  Misses            ?    23877           
  Partials          ?      378
Flag Coverage Δ Complexity Δ
#javascript 53.7% <ø> (?) 0 <ø> (?)
#phpunit 67.15% <100%> (?) 18777 <4> (?)
Impacted Files Coverage Δ Complexity Δ
apps/dav/lib/Files/BrowserErrorPagePlugin.php 90% <100%> (ø) 5 <2> (?)
apps/dav/lib/Connector/Sabre/FilesPlugin.php 87.07% <100%> (ø) 77 <2> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 27d12b0...039230d. Read the comment docs.

@DeepDiver1975 DeepDiver1975 force-pushed the bugfix/use-throwable-not-exception-as-typehint branch from 32b9ac6 to 039230d Compare July 2, 2019 06:58
@DeepDiver1975
Copy link
Copy Markdown
Member Author

@patrickjahns addressed - THX

@patrickjahns patrickjahns self-requested a review July 2, 2019 15:58
Copy link
Copy Markdown
Contributor

@patrickjahns patrickjahns left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@DeepDiver1975 DeepDiver1975 merged commit fd8c3d7 into master Jul 3, 2019
@delete-merged-branch delete-merged-branch Bot deleted the bugfix/use-throwable-not-exception-as-typehint branch July 3, 2019 06:33
@DeepDiver1975
Copy link
Copy Markdown
Member Author

backport #35748

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.

3 participants