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

Catch session unavailable exception #30141

Merged
merged 1 commit into from
Feb 2, 2018
Merged

Conversation

tomneedham
Copy link
Contributor

Description

Throw the correct excpetion so that it is properly caught on the session desctructor.

Motivation and Context

When the session destructor is called this exception can bubble up unnecessarily - its the wrong type.

How Has This Been Tested?

unit tests written

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • [] I have added tests to cover my changes.
  • [] All new and existing tests passed.

Copy link
Contributor

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

👍

@PVince81
Copy link
Contributor

@ownclouders please rebase

@codecov
Copy link

codecov bot commented Jan 29, 2018

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #30141   +/-   ##
=========================================
  Coverage          ?    60.8%           
  Complexity        ?    18519           
=========================================
  Files             ?     1092           
  Lines             ?    61195           
  Branches          ?        0           
=========================================
  Hits              ?    37211           
  Misses            ?    23984           
  Partials          ?        0
Impacted Files Coverage Δ Complexity Δ
lib/private/Session/Memory.php 86.95% <0%> (ø) 11 <0> (?)

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 1dbc499...b17d059. Read the comment docs.

@tomneedham
Copy link
Contributor Author

Fixed test. Lets see if 2 out of 3 CI services managed to die again this time -.-

@tomneedham
Copy link
Contributor Author

@ownclouders please rebase

@phil-davis
Copy link
Contributor

phil-davis commented Feb 2, 2018

@tomneedham you have to command him/her/it first - then saying please is optional

@phil-davis
Copy link
Contributor

@ownclouders rebase

@ownclouders
Copy link
Contributor

Hey! I'm GitMate.io! This pull request is being rebased automatically. Please DO NOT push while rebase is in progress or your changes would be lost permanently ⚠️

@ownclouders
Copy link
Contributor

Automated rebase with GitMate.io was successful! 🎉

@tomneedham
Copy link
Contributor Author

then saying please is optional

🙄

@phil-davis
Copy link
Contributor

Backport stable10 #30347

@lock
Copy link

lock bot commented Aug 1, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants