fix(errors): add error middleware that handles erorrs thrown, based o…

[dtfiedler]

…n their types and returns proper status codes

This type of middleware is particularly useful for this problem. We have various instances in which warp throws errors when failing to evaluate a contract state, and we want to parse those properly and return the correct status code when they occur. Right now we over thrown 503 errors, which suggest to clients that the app failed to process, when it's more likely they are requesting contracts that have not been mined yet or do not adhere to specs defined by warp. By using the middleware, we catch any errors thrown by our handlers, and properly set status codes based on the type of error.

It's intentionally added after the logger middleware so it has access to the logger (with trace id, request path, etc.) when logging out the error message before returning.

TODO:

• add tests that TX_NOT_FOUND errors in warp - occurs for new contracts deployed that have not been mined OR one that has a source contract that does not exist

Testing

Missing source code tx

Before

curl -iL http://localhost:3000/v1/contract/9Uv9wmhL19qkBJnaQZJ9rM-b59JvIA82dBLA2SJK0GY

HTTP/2 503 
content-type: text/plain; charset=utf-8
content-length: 152
date: Wed, 20 Sep 2023 19:11:46 GMT
cache-control: no-cache
x-cache: Error from cloudfront
via: 1.1 bd96da9a6020f2a5d5499cedf78d68f2.cloudfront.net (CloudFront)
x-amz-cf-pop: DEN52-P2
x-amz-cf-id: _TKd0HVwtwalxgUhyr0KKsmfNhuMk-voiKFgPEcmFcgpy3lfhXkY_Q==
x-xss-protection: 1; mode=block
x-content-type-options: nosniff
strict-transport-security: max-age=3600; includeSubDomains; preload
vary: Origin

Failed to fetch contract: 9Uv9wmhL19qkBJnaQZJ9rM-b59JvIA82dBLA2SJK0GY. Unable to retrieve tx npsNpXNps3wf0o15tQIW-AoXx7IBQbiURpwnWJhgdLk. 404.

After

❯ curl -iL http://localhost:3000/v1/contract/9Uv9wmhL19qkBJnaQZJ9rM-b59JvIA82dBLA2SJK0GY

HTTP/1.1 404 Not Found
Vary: Origin
Access-Control-Allow-Origin: 
Content-Type: text/plain; charset=utf-8
Content-Length: 81
Date: Wed, 20 Sep 2023 19:12:50 GMT
Connection: keep-alive
Keep-Alive: timeout=5

Unable to retrieve tx npsNpXNps3wf0o15tQIW-AoXx7IBQbiURpwnWJhgdLk. 404.

Non-existent contract

Before

❯ curl -iL http://localhost:3000/v1/contract/kbtXub0JcZYfBn7-WUgkFQjKmZb4y5DY2nT8WovBhWY
HTTP/2 503 
content-type: text/plain; charset=utf-8
content-length: 71
date: Wed, 20 Sep 2023 19:16:41 GMT
cache-control: no-cache
x-cache: Error from cloudfront
via: 1.1 8a50fe9452625079391cf0ce7a3e0c56.cloudfront.net (CloudFront)
x-amz-cf-pop: DEN52-P2
x-amz-cf-id: R895mZrcxd-qW-KheKmgD7WHnxtITXNjjKXPJefR37eg7Nwnc3ho3w==
x-xss-protection: 1; mode=block
x-content-type-options: nosniff
strict-transport-security: max-age=3600; includeSubDomains; preload
vary: Origin

Failed to fetch contract: kbtXub0JcZYfBn7-WUgkFQjKmZb4y5DY2nT8WovBhWY. %

After

❯ curl -iL http://localhost:3000/v1/contract/kbtXub0JcZYfBn7-WUgkFQjKmZb4y5DY2nT8WovBhWY
HTTP/1.1 404 Not Found
Vary: Origin
Access-Control-Allow-Origin: 
Content-Type: text/plain; charset=utf-8
Content-Length: 18
Date: Wed, 20 Sep 2023 19:20:47 GMT
Connection: keep-alive
Keep-Alive: timeout=5

Contract not found

[arielmelendez]

Neat approach, though it may come with some tradeoffs. I like how it consolidates handling of sending the error response and makes handler code a bit more succinct and happy path focused. I think what you lose, however, is the ability for your child loggers in the handler, which may have helpful debugging context, to connect the error they CAN see in the handler code with the error they CAN'T see in the error handler. TraceIDs will alleviate that at debugging time when you've already found your needles in the haystack, but might be more difficult to find the needles you're looking for altogether.

[dtfiedler]

Neat approach, though it may come with some tradeoffs. I like how it consolidates handling of sending the error response and makes handler code a bit more succinct and happy path focused. I think what you lose, however, is the ability for your child loggers in the handler, which may have helpful debugging context, to connect the error they CAN see in the handler code with the error they CAN'T see in the error handler. TraceIDs will alleviate that at debugging time when you've already found your needles in the haystack, but might be more difficult to find the needles you're looking for altogether.

yeah - you lose that a bit - that’s where the type of errors and the messages you include in them would become more important. you can also still use try/catch in your handlers for logging purposes, then just propagate the error up the the middleware to determine status code. i’m not sure this approach would be very useful in upload-service, where our error paths are complex and debugging logs are critical for understanding where errors originate.