Remove sourcegraph.com fallback logic
Created by: felixfbecker
The logic to fallback to sourcegraph.com does not work properly and adds a lot of complexity that affects the entire browser extension.
Problems:
- It is implemented in the lowest layer, the GraphQL request function. Higher layers therefor don't know if a request succeeded by retrying on sourcegraph.com. This results in part of the app "just working" with the fallback and the other part thinking the repo exists properly on the configured instance, e.g. the "view on sourcegraph" button #3232 (closed)
-
performRequest()
may throw non-Error instances (the response object) depending on specific GraphQL response fields. The conditions for this are so complex that it is near impossible to know as a downstream consumer if a call may throw an Error (likeRepoNotFoundError
), throw the response object, or return the response object withnull
fields. Bugs from this are guaranteed and it makes proper error handling very difficult to verify. - Since exceptions are not type checked in TypeScript, this would be hard to refactor
- It doesn't make sense to clean up all the messy error handling in the browser extension if the error throwing is so messy (we would just code to the bad error flow, and have to change it all again later)
Proposal:
- We replace backend.ts with sane API client functions that do not do fancy fallback logic (see webapp), never inspect GraphQL fields and never throw non-Error instances
- The
queryGraphQL()
function should only throw if the request fails, it returns{ data, errors }
raw from GraphQL - The higher-level functions like
resolveRev()
inspect the result and can throw specific errors.
- The
- We clean up error handling to work with this predictable error flow
- Later, we can add fallback properly on the right layer: Checking explicitly once where the repo exists, then passing the repo location around.
cc @sqs