router icon indicating copy to clipboard operation
router copied to clipboard

tui polish: improve formatting of Uplink message on `ACCESS_DENIED`

Open abernix opened this issue 1 year ago β€’ 1 comments

Could we improve this output formatting in the event of there being an Uplink error? This just ends up looking a bit hard to read to me. (The message even has a newline \n in it which makes it even more challenging to read.)

Maybe we could have a presentation layer for errors with code and message properties?

$ APOLLO_KEY=my-key-that-ends-in-tapQ APOLLO_GRAPH_REF=current ./router
2022-08-11T10:48:45.008224Z  INFO apollo_router::executable: Apollo Router v0.15.1 // (c) Apollo Graph, Inc. // Licensed as ELv2 (https://go.apollo.dev/elv2)
2022-08-11T10:48:45.014625Z  INFO apollo_router::state_machine: transitioned to startup
2022-08-11T10:48:45.015796Z  INFO apollo_router::state_machine: transitioned to startup
2022-08-11T10:48:45.979291Z ERROR apollo_router::router: error downloading the schema from Uplink: UpLink { code: ACCESS_DENIED, message: "API key service:abernix-20220811-deleteme:●●●●●●●●●●●●●●●●●●tapQ cannot access current.\nNote: Consumer graph tokens cannot be used for managed federation." }
2022-08-11T10:48:46.259521Z ERROR apollo_router::router: error downloading the schema from Uplink: UpLink { code: ACCESS_DENIED, message: "API key service:abernix-20220811-deleteme:●●●●●●●●●●●●●●●●●●tapQ cannot access current.\nNote: Consumer graph tokens cannot be used for managed federation." }
2022-08-11T10:48:55.511225Z ERROR apollo_router::router: error downloading the schema from Uplink: UpLink { code: ACCESS_DENIED, message: "API key service:abernix-20220811-deleteme:●●●●●●●●●●●●●●●●●●tapQ cannot access current.\nNote: Consumer graph tokens cannot be used for managed federation." }

abernix avatar Aug 11 '22 10:08 abernix

Also, I'm assuming we're printing this message three times because we're retrying. In a REST HTTP world, this would probably be a 401 and reqwest wouldn't retry it (on account of 4xx-ness), but since this is GraphQL and HTTP 200 OK, we might need to special case ACCESS_DENIED as a non-retryable error?

I don't think there's any normal condition where this would work after failing the first time.

abernix avatar Aug 11 '22 11:08 abernix