Skip to content

Catch internal server error from remote on decline#35178

Merged
micbar merged 1 commit into
masterfrom
catch-exception-on-decline
May 24, 2019
Merged

Catch internal server error from remote on decline#35178
micbar merged 1 commit into
masterfrom
catch-exception-on-decline

Conversation

@VicDeo

@VicDeo VicDeo commented May 8, 2019

Copy link
Copy Markdown
Contributor

Description

Handle 500 exception on declining a share

Motivation and Context

Before 10.2 response for declining non-existing shares was 500
#34786 changes HTTP status from 500 to 410
But declining any non-existing share for OC lower than 10.2 still causing unhandled exception.

How Has This Been Tested?

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)

@VicDeo VicDeo added this to the development milestone May 8, 2019
@VicDeo VicDeo self-assigned this May 8, 2019
@VicDeo VicDeo force-pushed the catch-exception-on-decline branch from ca5dce3 to c1fbd89 Compare May 8, 2019 09:30
@codecov

codecov Bot commented May 8, 2019

Copy link
Copy Markdown

Codecov Report

Merging #35178 into master will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #35178      +/-   ##
============================================
- Coverage     65.54%   65.53%   -0.01%     
- Complexity    18647    18651       +4     
============================================
  Files          1218     1218              
  Lines         70546    70549       +3     
  Branches       1288     1288              
============================================
  Hits          46236    46236              
- Misses        23933    23936       +3     
  Partials        377      377
Flag Coverage Δ Complexity Δ
#javascript 53.69% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 66.89% <0%> (-0.01%) 18651 <0> (+4)
Impacted Files Coverage Δ Complexity Δ
...s/federatedfilesharing/lib/AppInfo/Application.php 44.96% <0%> (-0.93%) 23 <0> (+4)

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 8a33127...ea3f39c. Read the comment docs.

@VicDeo VicDeo force-pushed the catch-exception-on-decline branch from c1fbd89 to b2b0ea8 Compare May 8, 2019 10:00
@owncloud owncloud deleted a comment from codecov Bot May 8, 2019
@PVince81 PVince81 added the p2-high Escalation, on top of current planning, release blocker label May 9, 2019
@VicDeo VicDeo force-pushed the catch-exception-on-decline branch from b2b0ea8 to 95cff43 Compare May 13, 2019 20:45
$event->getRemoteId(),
$event->getShareToken()
);
} catch (\Exception $e) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Any chance to refine this exception into GuzzleHttpException or something similar that might be being thrown? It's unlikely that a BlaBlaBlaException with code 500 reaches this point, but we'll have problems detecting this scenario with this code. It's the typical thing that will take a long time to detect.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

changed to be \GuzzleHttp\Exception\ServerException

@VicDeo VicDeo force-pushed the catch-exception-on-decline branch from 95cff43 to 1c2c7fb Compare May 20, 2019 16:43
@VicDeo VicDeo force-pushed the catch-exception-on-decline branch from 1c2c7fb to 65a3e14 Compare May 21, 2019 13:05
@VicDeo VicDeo force-pushed the catch-exception-on-decline branch from 65a3e14 to ea3f39c Compare May 21, 2019 14:19
@micbar micbar merged commit d2f25d1 into master May 24, 2019
@delete-merged-branch delete-merged-branch Bot deleted the catch-exception-on-decline branch May 24, 2019 07:19
@micbar

micbar commented May 24, 2019

Copy link
Copy Markdown
Contributor

@VicDeo please backport.

@VicDeo

VicDeo commented May 24, 2019

Copy link
Copy Markdown
Contributor Author

Stable10: #35321

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

Labels

3 - To Review p2-high Escalation, on top of current planning, release blocker

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants