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 special handling for UnavailableException #1861

Merged
merged 2 commits into from
Jun 13, 2017
Merged

Conversation

farmdawgnation
Copy link
Member

Mailing List thread:
https://groups.google.com/forum/#!topic/liftweb/BHYjE5C-kIk

This code adds special handling for javax.servelet.UnavailableException.
This exception is an idiomatic way to signal a full abort to the Java
Application server running the application.

From now on, we will log and re-throw this exception if we see it.

Fixes #1843

This code adds special handling for javax.servelet.UnavailableException.
This exception is an idiomatic way to signal a full abort to the Java
Application server running the application.

Please see #1843 for more information.
@farmdawgnation
Copy link
Member Author

@fanf This PR resolves the issue you logged regarding javax.servlet.UnavailableException. Can you please take it out for a spin and verify that this fixes your issue completely before we ship it?

Copy link
Member

@Shadowfiend Shadowfiend left a comment

Choose a reason for hiding this comment

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

Looks good to me. Couple of off-hand remarks, but not a reason to not merge.

logger.error("------------------------------------------------------------------")
logger.error("------------------------------------------------------------------")
logger.error("------------------------------------------------------------------")
logger.error("********** Failed to Boot! An unavailable exception was thrown an all futher boot activities are aborted", unavailableException);
Copy link
Member

Choose a reason for hiding this comment

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

I might say “An UnavailableException was thrown”; either way, it should be “and” rather than “an”.

Copy link
Member Author

Choose a reason for hiding this comment

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

Aye.

logger.error("------------------------------------------------------------------")
logger.error("------------------------------------------------------------------")
logger.error("------------------------------------------------------------------")
logger.error("------------------------------------------------------------------")
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need the four lines of dashes? Feels like one should be more than enough……

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not super opinionated on this, personally, but I'm inclined to leave it as is for now and reconsider when we next touch this code.

@Shadowfiend Shadowfiend added this to the 3.1.0-RC1 milestone Jun 12, 2017
@fanf
Copy link

fanf commented Jun 13, 2017

This is ok with me, the behavior is what is needed. I'm not fan of the big error message, but I understand the need for consistancy.

@farmdawgnation
Copy link
Member Author

Yeah, I think we'll revisit that error message at some point in the future.

@farmdawgnation
Copy link
Member Author

Cool, then I think this is good to merge.

@Shadowfiend
Copy link
Member

Let us, as they say in the lands of Sekulampa, do it.

@Shadowfiend Shadowfiend merged commit f03789b into master Jun 13, 2017
@Shadowfiend Shadowfiend deleted the msf_issue_1843 branch June 13, 2017 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants