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

Improve client-side error response messages #75

Merged
merged 1 commit into from
Sep 18, 2017

Conversation

kellyi
Copy link
Contributor

@kellyi kellyi commented Sep 18, 2017

Overview

This PR adds some infrastructure to send more descriptive error messages back to the client and to catch other exceptions. I set it up by adding an ErrorHandler trait which conforms to akka-http's custom exception handling along with some case classes to match on some known exceptions we throw in the app -- for missing required fields & invalid raster ops. These are sent back as 400s; for other unforeseen exceptions we send back a 500 along with the exception message.

I also set this up to print the exception messages to the service console. I think we should configure these to log, too, but it seems like it may require some work to get LazyLogging to work with/supplant log4j, the latter of which is there via Spark.

We have #63 for configuring the logger.

Connects #69

Testing Instructions

  • get this branch, then ./scripts/server
  • send a variety of malformed requests to the server and verify that you get appropriate messages back and that you see the exceptions printed to the console

@rajadain
Copy link
Member

Testing independently seems to be working very well:

http --print HhBb :8090/run < SquareKmPpt.json.txt

POST /run HTTP/1.1
Accept: application/json, */*
Accept-Encoding: gzip, deflate
Connection: keep-alive
Content-Length: 447
Content-Type: application/json
Host: localhost:8090
User-Agent: HTTPie/0.9.9

{
    "input": {
        "operationType": "RasterGroupedAverage",
        "pixelIsArea": true,
        "polygon": [
            "{\"type\":\"MultiPolygon\",\"coordinates\":[[[[-75.17123123758348,39.94946918388007],[-75.1595030153462,39.94946918388007],[-75.1595030153462,39.95845957765271],[-75.17123123758348,39.95845957765271],[-75.17123123758348,39.94946918388007]]]]}"
        ],
        "polygonCRS": "LatLng",
        "rasterCRS": "ConusAlbers",
        "rasters": [],
        "zoom": 0
    }
}

HTTP/1.1 400 Bad Request
Content-Length: 29
Content-Type: text/plain; charset=UTF-8
Date: Mon, 18 Sep 2017 21:30:45 GMT
Server: akka-http/10.0.9

Missing required targetRaster
http --print HhBb :8090/run < SquareKmPpt.json.txt

POST /run HTTP/1.1
Accept: application/json, */*
Accept-Encoding: gzip, deflate
Connection: keep-alive
Content-Length: 498
Content-Type: application/json
Host: localhost:8090
User-Agent: HTTPie/0.9.9

{
    "input": {
        "operationType": "RasterGroupedAverage",
        "pixelIsArea": true,
        "polygon": [
            "{\"type\":\"MultiPolygon\",\"coordinates\":[[[[-75.17123123758348,39.94946918388007],[-75.1595030153462,39.94946918388007],[-75.1595030153462,39.95845957765271],[-75.17123123758348,39.95845957765271],[-75.17123123758348,39.94946918388007]]]]}"
        ],
        "polygonCRS": "LatLng",
        "rasterCRS": "ConusAlbers",
        "rasters": [],
        "targetRaster": "climatology-ppt-14-epsg5070",
        "zoom": 0
    }
}

HTTP/1.1 500 Internal Server Error
Content-Length: 132
Content-Type: text/plain; charset=UTF-8
Date: Mon, 18 Sep 2017 21:30:22 GMT
Server: akka-http/10.0.9

geotrellis.spark.io.package$LayerNotFoundError: Layer Layer(name = "climatology-ppt-14-epsg5070", zoom = 0) not found in the catalog

Going to test out in MMW now.

Copy link
Member

@rajadain rajadain left a comment

Choose a reason for hiding this comment

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

+1 tested. Installed in MMW and got this error on the front-end:

image

@rajadain
Copy link
Member

I agree that printing is fine for now, and we'll do logging properly with #63. Nice job using the Error Handler pattern of AKKA for this, and subclassing all errors from a custom GeoprocessingError. 👍

@rajadain rajadain assigned kellyi and unassigned rajadain Sep 18, 2017
- add custom exception handler + exception case classes
- propagate error messages back to the client with 400s if they're known
  errors & 500 if they're unknown
@kellyi
Copy link
Contributor Author

kellyi commented Sep 18, 2017

Thanks! It occurred to me that we don't need the values from some of pattern matches here:

https://github.com/WikiWatershed/mmw-geoprocessing/pull/75/files#diff-21f7ae43c2d12dffcf68e5c22e585faaR25

... so I'm going to squash a fixup to remove them if they're not used.

@kellyi kellyi force-pushed the ki/improve-error-reporting branch from cb5cc54 to 9a8b736 Compare September 18, 2017 21:53
@kellyi kellyi merged commit de66670 into develop Sep 18, 2017
@kellyi kellyi deleted the ki/improve-error-reporting branch September 18, 2017 21:58
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.

2 participants