bluesky-social / atproto

Social networking technology created by Bluesky
Other
6.07k stars 426 forks source link

type of error returned from com.atproto.repo.getRecord #1712

Open mackuba opened 11 months ago

mackuba commented 11 months ago

Describe the bug

There is a com.atproto.admin.getRecord endpoint, which looks up a specific record. If the record isn't found, it throws such error:

throw new InvalidRequestError('Record not found', 'RecordNotFound')

There is also the com.atproto.repo.getRecord endpoint, which does more or less the same thing, just with different output; and when it doesn't find the record, it throws this error:

throw new InvalidRequestError(`Could not locate record: ${uri}`)

which produces an {"error":"InvalidRequest"} JSON response instead of {"error":"RecordNotFound"}.

Shouldn't com.atproto.repo.getRecord also return a typed error RecordNotFound like the other method?

bnewbold commented 11 months ago

Yeah, that would make sense.

@foysalit can you bundle this change in with other admin refactors?

FWIW, there is a good chance that we will also start returning 404 and/or 410 status codes for some of these errors. Originally XRPC tried to never return 404 (because that was reserved to indicate that XRPC wasn't supported), but that has been dropped and we can use the expected status codes.

mackuba commented 11 months ago

Oooh, nice, that was bugging me too! It's kind of important for the client to be able to distinguish the case "you're calling something incorrectly and you need to fix the code" from "you're doing everything ok, but this thing is not there" in the error handling code.