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

=htc replace usage of RuleNotFoundException by introducing explicit result ADT #1424

Merged

Conversation

jrudolph
Copy link
Member

Before every attempt to parse an unknown (i.e. not modeled) header would
end up in throwing and catching an exception. This does not really make
sense as parboiled already provides for delivering these signals without
an exception.

Refs playframework/playframework#7822

@akka-ci akka-ci added validating PR that is currently being validated by Jenkins needs-attention Indicates a PR validation failure (set by CI infrastructure) and removed validating PR that is currently being validated by Jenkins labels Sep 14, 2017
@akka-ci
Copy link

akka-ci commented Sep 14, 2017

Test FAILed.

@jrudolph jrudolph force-pushed the jr/w/remote-header-parser-RuleNotFoundException branch from 74a8722 to 5170d5f Compare September 14, 2017 12:56
@jrudolph
Copy link
Member Author

Added missing mima filters.

@akka-ci akka-ci added validating PR that is currently being validated by Jenkins needs-attention Indicates a PR validation failure (set by CI infrastructure) and removed needs-attention Indicates a PR validation failure (set by CI infrastructure) validating PR that is currently being validated by Jenkins labels Sep 14, 2017
@akka-ci
Copy link

akka-ci commented Sep 14, 2017

Test FAILed.

@jrudolph jrudolph force-pushed the jr/w/remote-header-parser-RuleNotFoundException branch from 5170d5f to 501661e Compare September 14, 2017 13:18
@akka-ci akka-ci added validating PR that is currently being validated by Jenkins tested PR that was successfully built and tested by Jenkins and removed needs-attention Indicates a PR validation failure (set by CI infrastructure) validating PR that is currently being validated by Jenkins labels Sep 14, 2017
@akka-ci
Copy link

akka-ci commented Sep 14, 2017

Test PASSed.

@@ -0,0 +1,7 @@
# Changes to internal API
ProblemFilters.exclude[MissingClassProblem]("akka.http.impl.model.parser.HeaderParser$RuleNotFoundException$")
Copy link
Member

Choose a reason for hiding this comment

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

Can we ProblemFilters.exclude[Problem]("akka.http.impl.*")?

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 guess we can. It's somewhat interesting to see what has changed in the API but it's mostly distracting so I'll try.

@@ -62,18 +62,21 @@ private[http] class HeaderParser(

///////////////// DynamicRuleHandler //////////////

type Result = Either[ErrorInfo, HttpHeader]
type Result = HeaderParser.Result
Copy link
Member

Choose a reason for hiding this comment

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

I was confused why not just use HeaderParser.Result directly, but this is specifying the Result from DeliveryScheme.

It looks like type also accepts an override modifier, might be a nice way to make that explicit?

Copy link
Member Author

Choose a reason for hiding this comment

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

👍

case NonFatal(e) ⇒ ErrorInfo.fromCompoundString(e.getMessage)
}
}
def ruleNotFound(ruleName: String): Result = HeaderParser.RuleNotFound
Copy link
Member

Choose a reason for hiding this comment

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

👍 nice!

… result ADT

Before every attempt to parse an unknown (i.e. not modeled) header would
end up in throwing and catching an exception. This does not really make
sense as parboiled already provides for delivering these signals without
an exception.

Refs playframework/playframework#7822
@jrudolph jrudolph force-pushed the jr/w/remote-header-parser-RuleNotFoundException branch from 501661e to b181ffa Compare September 18, 2017 08:57
@akka-ci akka-ci added validating PR that is currently being validated by Jenkins tested PR that was successfully built and tested by Jenkins and removed tested PR that was successfully built and tested by Jenkins validating PR that is currently being validated by Jenkins labels Sep 18, 2017
@akka-ci
Copy link

akka-ci commented Sep 18, 2017

Test PASSed.

@jrudolph jrudolph merged commit 32071d8 into akka:master Sep 18, 2017
@jrudolph jrudolph deleted the jr/w/remote-header-parser-RuleNotFoundException branch September 18, 2017 09:07
@jrudolph jrudolph added this to the 10.0.11 milestone Sep 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tested PR that was successfully built and tested by Jenkins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants