Considerations for error pathways for endpoints

[TangoYankee]

Description

The Open API documentation covers the happy paths for the endpoints. We should start to consider how each endpoint may find itself in an error state. From there we can first document in OpenAPI how the application will behave in these errors states. Then, we implement code to handle these error states.

Discussion strategy

This top-level comment will capture each endpoint and the error paths we wish to capture. Ideas can be added through comments and discussed in replies. As we reach a consensus, this top-level comment will be updated to reflect this consensus. Once we feel we have settled on a list of error states to cover, we can break the work into separate tickets.

Consensus

Type Validation

There hasn't been a strong preference shown for a validator library, other than wanting to coalesce around a common library. But because drizzle provides a drizzle-zod library and zod appears to be a mature library in this space, we should leverage zod here.

Endpoints

boroughs

/boroughs

• Documented errors
  • 400: Bad request
  • 500: Internal Server Error
• Implemented errors
  • 400, caused by malformed requests and handled automatically by Nest
  • 500, caused by uncaught errors and handled automatically by Nest

Land uses

/land-uses

• Documented errors
  • 400: Bad request
  • 500: Internal Server Error
• Implemented errors
  • 400, caused by malformed requests and handled automatically by Nest
  • 500, caused by uncaught errors and handled automatically by Nest

Tax lots

tax-lots/<bbl>

• Documented errors
  • 400: Bad request
  • 404: Not Found
  • 500: Internal Server Error
• Implemented errors
  • 400, caused by malformed requests and handled automatically by Nest
  • 400, caused by request parameter of invalid type or parameter and handled with bespoke enforcement
  • 404, caused by the requested id having no results and handled by throwing an error when there are no results
  • 500, caused by uncaught errors and handled automatically by Nest

tax-lots/<bbl>/geojson

• Documented errors
  • 400: Bad request
  • 404: Not Found
  • 500: Internal Server Error
• Implemented errors
  • 400, caused by malformed requests and handled automatically by Nest
  • 400, caused by request parameter of invalid type or parameter and handled with bespoke enforcement
  • 404, caused by the requested id having no results and handled by throwing an error when there are no results
  • 500, caused by uncaught errors and handled automatically by Nest

tax-lots/<bbl>/zoning-districts

• Documented errors
  • 400: Bad request
  • 404: Not Found
  • 500: Internal Server Error
• Implemented errors
  • 400, caused by malformed requests and handled automatically by Nest
  • 400, caused by request parameter of invalid type or parameter and handled with bespoke enforcement
  • 404, caused by the requested tax lot id not existing and handled by throwing an error
    • The endpoint should first check for the existence of the tax lot by doing a standard findFirst. If there is a result, it can proceed to find the zoning districts for that tax lot
  • 500, caused by uncaught errors and handled automatically by Nest

tax-lots/<bbl>/zoning-districts/classes

• Documented errors
  • 400: Bad request
  • 404: Not Found
  • 500: Internal Server Error
• Implemented errors
  • 400, caused by malformed requests and handled automatically by Nest
  • 400, caused by request parameter of invalid type or parameter and handled with bespoke enforcement
  • 404, caused by the requested tax lot id not existing and handled by throwing an error
    • The endpoint should first check for the existence of the tax lot by doing a standard findFirst. If there is a result, it can proceed to find the zoning district classes for that tax lot
  • 500, caused by uncaught errors and handled automatically by Nest

tax-lots/<z>/<x>/<y>.pbf

• Documented errors
  • 400: Bad request
  • 500: Internal Server Error
• Implemented errors
  • 400, caused by malformed requests and handled automatically by Nest
  • 400, caused by request parameter of invalid type or parameter and handled with bespoke enforcement
  • 500, caused by uncaught errors and handled automatically by Nest

Zoning districts

zoning-districts/<uuid>

• Documented errors
  • 400: Bad request
  • 404: Not Found
  • 500: Internal Server Error
• Implemented errors
  • 400, caused by malformed requests and handled automatically by Nest
  • 400, caused by request parameter of invalid type or parameter and handled with bespoke enforcement
  • 404, caused by the requested id having no results and handled by throwing an error when there are no results
  • 500, caused by uncaught errors and handled automatically by Nest

zoning-districts/<uuid>/classes

• Documented errors
  • 400: Bad request
  • 404: Not Found
  • 500: Internal Server Error
• Implemented errors
  • 400, caused by malformed requests and handled automatically by Nest
  • 400, caused by request parameter of invalid type or parameter and handled with bespoke enforcement
  • 404, caused by the requested zoning-district id not existing and handled by throwing an error
    • The endpoint should first check for the existence of the zoning district by doing a standard findFirst. If there is a result, it can proceed to find the classes for the zoning district
  • 500, caused by uncaught errors and handled automatically by Nest

zoning-districts/<z>/<x>/<y>.pbf

• Documented errors
  • 400: Bad request
  • 500: Internal Server Error
• Implemented errors
  • 400, caused by malformed requests and handled automatically by Nest
  • 400, caused by request parameter of invalid type or parameter and handled with bespoke enforcement
  • 500, caused by uncaught errors and handled automatically by Nest

Zoning district classes

zoning-district-classes

• Documented errors
  • 400: Bad request
  • 500: Internal Server Error
• Implemented errors
  • 400, caused by malformed requests and handled automatically by Nest
  • 500, caused by uncaught errors and handled automatically by Nest

zoning-district-classes/category-colors

• Documented errors
  • 400: Bad request
  • 500: Internal Server Error
• Implemented errors
  • 400, caused by malformed requests and handled automatically by Nest
  • 500, caused by uncaught errors and handled automatically by Nest

zoning-district-classes/<id>

• Documented errors
  • 400: Bad request
  • 404: Not Found
  • 500: Internal Server Error
• Implemented errors
  • 400, caused by malformed requests and handled automatically by Nest
  • 400, caused by request parameter of invalid type or parameter and handled with bespoke enforcement
  • 404, caused by the requested id having no results and handled by throwing an error when there are no results
  • 500, caused by uncaught errors and handled automatically by Nest

Schemas

components:
  responses:
    Bad Request:
      description: Invalid client request.
      content:
        application/json:
          schema:
            $ref: '#/components/schemas/Error'
    Not Found:
      description: Requested resource does not exist or is unavailable.
      content:
        application/json:
          schema:
            $ref: '#/components/schemas/Error'
    Internal Server Error:
      description: Server side error.
      content:
        application/json:
          schema:
            $ref: '#/components/schemas/Error'

  schemas:
    # Schema for error response body (following the Nest.js general structure)
    Error:
      type: object
      properties:
        statusCode:
          type: number
        message:
          type: string
       error:
          type: string
      required:
        - statusCode
        - message

Messages

The messages returned from the api for each type of error (there may be different messages for each status code, depending on the cause)

• 400 from malformed request
  • Let Nest automatically send error with message
• 400 from bad url parameter type or format
  • "Invalid request parameter data type or format"
• 404 from specified resource id not existing
  • "Not found"
• 500 catch all
  • Let Nest automatically send error with message
• 500 from database failure
  • "Error while retrieving data"

[TangoYankee]

Our open api generator tool, redocly, recommends that every endpoint return a 4xx series code. This makes sense. In practice, client request errors can derive from parts of the http request other than the url. MDN mentions "malformed request syntax, invalid request message framing, or deceptive request routing" as possible sources of 400 errors. Consequently, even endpoints that do not have dynamic path or query parameters can still have client errors.

To account for these errors, we should add 400 responses with a generic 'Bad Request' response to all of the endpoint Open API documentation. When making the Open API documentation, we can create a Bad Request component and reuse it for every endpoint. See the reusing-responses section of describing responses. Following the structure of the example, a 'Bad Request' response would build off a generic 'Error' response, like so:

components:
  responses:
    Bad Request:
      description: The request structure was invalid.
      content:
        application/json:
          schema:
            $ref: '#/components/schemas/Error'

  schemas:
    # Schema for error response body
    Error:
      type: object
      properties:
        code:
          type: number
        message:
          type: string
      required:
        - code
        - message

Implementation of this response should already be handled by Nest JS. 400 codes for bad requests are well-defined and Nest JS is generally designed to handle common cases like this.

[TangoYankee]

Routes that do accept path parameters may receive parameters that do not meet the expected type and/or format. In these cases, they should return a 400 that more specifically states that an invalid path parameter was passed. The affected endpoints are:

• tax-lots/<bbl>
• tax-lots/<bbl>/geojson
• tax-lots/<bbl>/zoning-districts
• tax-lots/<bbl>/zoning-districts/classes
• tax-lots/<z>/<x>/<y>.pbf
• zoning-districts/<uuid>
• zoning-districts/<uuid>/classes
• zoning-districts/<z>/<x>/<y>.pbf

In Open API, the Bad Request response is still a valid structure for these routes and no additional documentation will be necessary.

In Nest, we should use joi.js to validate that numbers and uuid path parameters are the correct data types and formats. When they are not, return a 400 with a message indicating that a path parameter was of an invalid data type or format.

[TangoYankee]

Routes that do accept path parameters may receive requests that are valid but no resource exists at that endpoint. These endpoints fall into two categories- those that make requests to the database and those that redirect to tilesets. The database routes stay within the application and the application has direct control over the response. These endpoints are:

• tax-lots/<bbl>
• tax-lots/<bbl>/geojson
• tax-lots/<bbl>/zoning-districts
• tax-lots/<bbl>/zoning-districts/classes
• zoning-districts/<uuid>
• zoning-districts/<uuid>/classes

The api should return a 404 for these routes. In Open API, we should have a response of structure:

 components:
  responses:
    Not Found:
      description: The requested resource was not found.
      content:
        application/json:
          schema:
            $ref: '#/components/schemas/Error'

The tileset routes redirect to our S3 bucket provider. When the targeted endpoint resource does not exist, the bucket returns a 403 Access Denied with the following XML response.

<Error>
     <Code>AccessDenied</Code>
     <Message></Message>
     <BucketName</BucketName>
     <RequestId></RequestId>
     <HostId></HostId>
</Error>

Because this does not follow the Error schema, the schema should be built using a new generic with xml representations

 components:
  responses:
    S3 Access Denied:
      description: The requested resource was not found in the associated S3 bucket.
      content:
        application/xml:
          schema:
             $ref: '#/components/schemas/BucketError'
   schemas:
   # Schema for error response body
    BucketError:
      type: object
      properties:
        code:
          type: string
        message:
          type: string
        bucketName
          type: string
        requestId
          type: string
        hostId
          type: string
      xml
         name: error
      required:
        - code
        - message
        - bucketName
        - requestId
        - hostId

This applies to the following tileset routes:

• tax-lots/<z>/<x>/<y>.pbf
• zoning-districts/<z>/<x>/<y>.pbf

[TangoYankee]

@bmarchena @dhochbaum-dcp @horatiorosa @TylerMatteo Any thoughts on either the existing comments or things that haven't been considered yet?

[horatiorosa]

joi.dev reminds me of validation we use in EDDE called YUP. Would that potentially be useful here?

[TylerMatteo]

It also reminds me of Zod :/ which @TangoYankee and I have talked about adopting. We truly are spoiled for choice when it comes to runtime validation libraries, but it would be ideal to coalesce around one sooner rather than later.

[TangoYankee]

I don't actually have an opinion on which to use. I just picked one. This sounds like we need an executive decision to just pick one @TylerMatteo

[horatiorosa]

IMHO, it would be nice if we just used one, instead of using this one in this app and that one in the other app but this different app uses some other thing. Unless y'all think it's better to not have all our validation eggs in one basket?

[TangoYankee]

If we use zod, then we can try the drizzle-zod package

[TylerMatteo]

Let's stick with Zod for now. We can get around to switching out Yup in EDDE eventually 😄

[TylerMatteo]

@TangoYankee This is really great, thank you for documenting your deep dive on the topic. I'll try to organize my thoughts as best as I can below:

400 responses

I'm sold on buying into Redocly rule of all requests having a 4xx response type, even for requests where there are no path parameters. I'm thinking routes with no path params would just have a 400 response type. For routes with with path params, do you think we should have 404 and 400 or just 404? It seems to be that technically we should - 400 if the request is invalid and 404 if it is valid but we just couldn't find a record for the supplied params.

Another thought I had is how do 5xx responses fit into this? This is maybe something we can table for now but I'm a little surprised redoc's linter requires 4xx but not 5xx. After all, if one of our GET endpoints throws an error because the database is down, that's a 5xx, not 4xx.

Shape of (non-XML) error responses

I think you have the right idea already - I think we should lean into the built in HTTP Exceptions supplied by Nest and design our OpenAPI schemas accordingly.

XML errors

I'm fine with implementing the XML error that comes back from DO like you're doing above, but this is something we should revisit. I'm not sure that we're actually "responsible" for describing the error that comes back from DO. I think that error is actually seen as not coming from our API because it's the result after a redirect. As far as our API is concerned:

• The redirect success response code is the end of the transaction.
• If the request failed for some other reason (such as the "malformed request syntax, invalid request message framing, or deceptive request routing" examples), the response would be our generic 400, not the XML from DO.

[TangoYankee]

I think we should lean into the built in HTTP Exceptions supplied by Nest

Comparing the template error above to what Nest generally returns, it seems we'd need to change code to statusCode and add an optional error field:

  schemas:
    # Schema for error response body
    Error:
      type: object
      properties:
        code:
          type: number
        message:
          type: string
       error:
          type: string
      required:
        - code
        - message

[TangoYankee]

For XML errors on the tilesets, I'm fine with only describing the 400s that may happen as part of trip to the api

[TangoYankee]

@TylerMatteo, should we use 502 Bad Gateway when calls to the database fail? I'm not sure whether calls to the db would be considered a third-party service

[TangoYankee]

For the below endpoints, we are finding a table of data related to a specific row of another table. In these cases, there may be no results either because 1) The specified row does have related data in the 'child' table or because 2) The specified row does not exist. I think there is is value in specifying which of these two causes occurred. This can be accomplished by first querying for a count of the rows with the specified id and throwing a 404 when the count is 0.

Arguably, the first cause doesn't need to throw a 404 at all. For example, if there are no zoning-districts for a specified tax lot, that is the correct answer; it is not an error. We should actually only error when the specified tax lot does not exist.

Affected endpoints

• tax-lots/<bbl>/zoning-districts
• tax-lots/<bbl>/zoning-districts/classes
• zoning-districts/<uuid>/classes

[TangoYankee]

From white-board discussion yesterday, a database connection does not count as a gateway and a 500 is sufficient. Also, endpoints that return a related table should only 404 when the specified row in the original table does not exist. For example, tax-lots/<bbl>/zoning-districts should 404 when the specified tax lot does not exist. However, it should return an empty array if the bbl does exist but there are no zoning districts associated with it.

[TangoYankee]

@TylerMatteo I experimented with how to make the exceptions reusable. See the error folder to see an example InvalidRequestParameterException and its use in the tax lot service.