You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jena.apache.org by ajs6f <gi...@git.apache.org> on 2017/04/17 14:23:23 UTC
[GitHub] jena pull request #241: JENA-1321: Exception rewrapping in HttpQuery masks e...
GitHub user ajs6f opened a pull request:
https://github.com/apache/jena/pull/241
JENA-1321: Exception rewrapping in HttpQuery masks error response from the server
It's a bit odd-feeling to couple `HttpException` so tightly, but given that it is an Atlas type, I don't feel terrible about it.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ajs6f/jena JENA-1321
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/jena/pull/241.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #241
----
commit 2a8257d0ef0799464d1f09f6badc9e7bf50c48fd
Author: ajs6f <aj...@apache.org>
Date: 2017-04-17T14:21:34Z
JENA-1321: Exception rewrapping in HttpQuery masks error response from the server
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena issue #241: JENA-1321: Exception rewrapping in HttpQuery masks error re...
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on the issue:
https://github.com/apache/jena/pull/241
No, actually, I did not start with your code-- if you look at what I did, I added fields and accessors and a constructor to `QueryExceptionHTTP` instead as we discussed. You need do nothing further. I will merge this in and it will appear in our next release. (In the meantime, you can just `git pull` the changes from the main repo (once I merge them).)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #241: JENA-1321: Exception rewrapping in HttpQuery masks e...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/241#discussion_r112040546
--- Diff: jena-arq/src/main/java/org/apache/jena/sparql/engine/http/QueryExceptionHTTP.java ---
@@ -63,11 +66,21 @@ public QueryExceptionHTTP(int responseCode)
public int getResponseCode() { return responseCode ; }
- /** The messge for the reason for this exception
+ /** The message for the reason for this exception
* @return message
*/
public String getResponseMessage() { return responseMessage ; }
+ /** The response for this exception
+ * @return response
+ */
+ public String getResponse() { return response ; }
+
+ /** The status line for the response for this exception
+ * @return status line
--- End diff --
"The status line for the response if available from HTTP"
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #241: JENA-1321: Exception rewrapping in HttpQuery masks e...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/241#discussion_r112040558
--- Diff: jena-arq/src/main/java/org/apache/jena/sparql/engine/http/QueryExceptionHTTP.java ---
@@ -63,11 +66,21 @@ public QueryExceptionHTTP(int responseCode)
public int getResponseCode() { return responseCode ; }
- /** The messge for the reason for this exception
+ /** The message for the reason for this exception
* @return message
*/
public String getResponseMessage() { return responseMessage ; }
+ /** The response for this exception
+ * @return response
--- End diff --
Note that this can be null if the original exception was not from the HTTP protocol, but from e.g. an IOException why doing an HTTP operation.
"The response for this exception if available from HTTP"
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena issue #241: JENA-1321: Exception rewrapping in HttpQuery masks error re...
Posted by rmorrise <gi...@git.apache.org>.
Github user rmorrise commented on the issue:
https://github.com/apache/jena/pull/241
Code looks good!
I am new to git, but if I understood correctly, you took my change plus adding the responseCode, but did not take my changes to TestService. I think this will fail the unit tests? Please correct me if I am reading the diff wrong.
Do I need to do anything for the changes to be applied to the main repo?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena issue #241: JENA-1321: Exception rewrapping in HttpQuery masks error re...
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on the issue:
https://github.com/apache/jena/pull/241
The tests pass fine, because as @afs prescribed, we aren't changing the wrapping of exceptions, just adding useful information to "outer" ones.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena issue #241: JENA-1321: Exception rewrapping in HttpQuery masks error re...
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on the issue:
https://github.com/apache/jena/pull/241
@rmorrise, will this suit you?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #241: JENA-1321: Exception rewrapping in HttpQuery masks e...
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f closed the pull request at:
https://github.com/apache/jena/pull/241
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena issue #241: JENA-1321: Exception rewrapping in HttpQuery masks error re...
Posted by rmorrise <gi...@git.apache.org>.
Github user rmorrise commented on the issue:
https://github.com/apache/jena/pull/241
My mistake! I didn't notice the call to httpEx.getCause() in the constructor.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---