Skip to content

Commit

Permalink
Add optional url argument to didEncounterErrors hook (#242)
Browse files Browse the repository at this point in the history
In previous versions of RESTDataSource, the URL of the request was
available on the Request object passed in to the hook. The Request
object is no longer passed as an argument, so this restores the
availability of the url to the hook.

This is optional for now in order to keep this change forward compatible
for existing this.didEncounterErrors call sites in userland code. In the
next major version, this might become a required parameter.
  • Loading branch information
trevor-scheer authored Aug 24, 2023
1 parent d2f0f5d commit dfb8bcc
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 2 deletions.
9 changes: 9 additions & 0 deletions .changeset/gorgeous-timers-relax.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
'@apollo/datasource-rest': minor
---

Add optional `url` parameter to `didEncounterErrors` hook

In previous versions of `RESTDataSource`, the URL of the request was available on the `Request` object passed in to the hook. The `Request` object is no longer passed as an argument, so this restores the availability of the `url` to the hook.

This is optional for now in order to keep this change forward compatible for existing `this.didEncounterErrors` call sites in userland code. In the next major version, this might become a required parameter.
1 change: 1 addition & 0 deletions cspell-dict.txt
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,5 @@ pino
revalidates
singletonizes
unmock
userland
withrequired
9 changes: 7 additions & 2 deletions src/RESTDataSource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,12 @@ export abstract class RESTDataSource {
request: FetcherRequestInit,
): ValueOrPromise<CacheOptions | undefined>;

protected didEncounterError(_error: Error, _request: RequestOptions) {
protected didEncounterError(
_error: Error,
_request: RequestOptions,
// TODO(v7): this shouldn't be optional in a future major version
_url?: URL,
) {
// left as a no-op instead of an unimplemented optional method to avoid
// breaking an existing use case where one calls
// `super.didEncounterErrors(...)` This could be unimplemented / undefined
Expand Down Expand Up @@ -544,7 +549,7 @@ export abstract class RESTDataSource {
},
};
} catch (error) {
this.didEncounterError(error as Error, outgoingRequest);
this.didEncounterError(error as Error, outgoingRequest, url);
throw error;
}
});
Expand Down
37 changes: 37 additions & 0 deletions src/__tests__/RESTDataSource.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2193,6 +2193,43 @@ describe('RESTDataSource', () => {
message: 'I replaced the error entirely',
});
});

it('is called with the url', async () => {
let urlFromDidEncounterError: URL | undefined = undefined;
const dataSource = new (class extends RESTDataSource {
override baseURL = 'https://api.example.com';

getFoo() {
return this.get('foo');
}

override didEncounterError(
_: Error,
__: RequestOptions,
url?: URL,
) {
urlFromDidEncounterError = url;
}
})();

nock(apiUrl)
.get('/foo')
.reply(
500,
{
errors: [{ message: 'Houston, we have a problem.' }],
},
{ 'content-type': 'application/json' },
);

try {
await dataSource.getFoo();
} catch {}

expect((urlFromDidEncounterError as any).toString()).toMatch(
'https://api.example.com/foo',
);
});
});
});
});
Expand Down

0 comments on commit dfb8bcc

Please sign in to comment.