feat: implement client to abstract bridge calls#226
Conversation
| UpdateJobParams, | ||
| } from './bridgeClient.types'; | ||
|
|
||
| export class BridgeClient extends BaseHttpClient { |
There was a problem hiding this comment.
Nit: should have a generic interface, with a specific implementation.
e.x. BridgeClient would be the interface, and the implementation would be OsoBridgeClient
There was a problem hiding this comment.
Good call, that'll make it easy to swap in a different bridge; added an interface and a switched the previous class to a concrete implementation for OSO!
pranishnepal
force-pushed
the
WCN-742
branch
2 times, most recently
from
c77331f to
d7d4d1f
Compare
| throw new ConflictError(error.response?.body.message); | ||
| default: | ||
| throw new Error( | ||
| `${ctx}${error.status ? ` [${error.status}]` : ''}${ |
There was a problem hiding this comment.
the default case now includes the internal URL in the error message, which surfaces in details on 500 responses to clients. the old keyProviderClient threw just the message for 500s.
There was a problem hiding this comment.
Good catch, i figured it'd be fine since it's error messaging, but i reverted to original way + the className to differentiate. Added a test case to validate also
| async submit(params: SubmitParams): Promise<SubmitResponse> { | ||
| const path = params.path.startsWith('/') ? params.path : `/${params.path}`; | ||
| const headers: Record<string, string> = { | ||
| 'X-OSO-Source': params.sources.join(','), |
There was a problem hiding this comment.
this needs to handle "user,backup" without spaces.
| const response = await this.call('get', `${this.url}/jobs`, { query }); | ||
|
|
||
| const jobs: BridgeJobResponse[] = []; | ||
| for (const job of response.body?.jobs ?? []) { |
There was a problem hiding this comment.
response.body?.jobs ?? [] silently returns empty on a malformed envelope.
There was a problem hiding this comment.
Good point, updated to throw when jobs isn't available. Added a test case also. Thank you!
This PR implements a client for the bridge service, which will be used by the master Bitgo Express server to forward certain requests to the bridge service. A base HTTP Client has been implemented and the the existing keyProvider has been refactored to extend it as well. Tests have been added for the new client and existing tests for the keyProvider have been updated whereby necessary(error handling changes). Ticket: WCN-743
zahin-mohammad
left a comment
zahin-mohammad
left a comment
There was a problem hiding this comment.
will defer to @pranavjain97 's review
3 participants
This PR implements a client for the bridge service, which will be used by the master Bitgo Express server to forward certain requests to the bridge service.
A base HTTP Client has been implemented and the the existing keyProvider has been refactored to extend it as well.
Tests have been added for the new client and existing tests for the keyProvider have been updated whereby necessary(error handling changes).
Ticket: WCN-743