Skip to content

feat: Errors from gax layer#1090

Merged
danieljbruce merged 23 commits intogoogleapis:mainfrom
danieljbruce:errors-from-gax-layer
Jul 4, 2022
Merged

feat: Errors from gax layer#1090
danieljbruce merged 23 commits intogoogleapis:mainfrom
danieljbruce:errors-from-gax-layer

Conversation

@danieljbruce
Copy link
Contributor

This PR contains tests for ensuring that errors which pass through the gax layer are presented to the user properly using unit tests from a grpc service.

@danieljbruce danieljbruce requested a review from a team as a code owner May 19, 2022 18:45
@danieljbruce danieljbruce requested a review from a team May 19, 2022 18:45
@product-auto-label product-auto-label bot added size: m Pull request size is medium. api: bigtable Issues related to the googleapis/nodejs-bigtable API. labels May 19, 2022
@danieljbruce danieljbruce marked this pull request as draft May 19, 2022 18:45
@danieljbruce danieljbruce changed the title Errors from gax layer feat: Errors from gax layer May 19, 2022
@product-auto-label product-auto-label bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels May 20, 2022
@product-auto-label product-auto-label bot added size: m Pull request size is medium. and removed size: l Pull request size is large. labels May 20, 2022
@product-auto-label product-auto-label bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels May 24, 2022
// netServer.listen(port);
netServer.listen(`localhost:${port}`);
}
describe('Ensure server shuts down properly when destroyed', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I add a before hook with the following in it:

checkPort(inputPort, false, done);

the assert.notEqual(err.code, 'EADDRINUSE'); fails, but I am not sure why at this time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed the way this worked entirely to use a third party dev dependency.

@danieljbruce danieljbruce marked this pull request as ready for review June 14, 2022 17:08
…into errors-from-gax-layer

# Conflicts:
#	package.json
@danieljbruce danieljbruce added the owlbot:run Add this label to trigger the Owlbot post processor. label Jun 17, 2022
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Jun 17, 2022
Copy link

@bcoe bcoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a nit, I would always opt for an async/await appraoch, rather than Promise.then these days.

const inputPort = '1234';
let server: MockServer;
function checkPort(port: string, inUse: boolean, callback: () => void) {
tcpPortUsed.check(parseInt(port), 'localhost').then((isInUse: boolean) => {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my advice would be to always use an async/await interface when possible, I think you could replace this with:

await checkPort()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It actually looks like this now:

  async function checkPort(port: string, inUse: boolean, callback: () => void) {
    const isInUse: boolean = await tcpPortUsed.check(
      parseInt(port),
      'localhost'
    );
    assert.strictEqual(isInUse, inUse);
    callback();
  }

@danieljbruce danieljbruce added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 20, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 20, 2022
@danieljbruce danieljbruce added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 20, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 20, 2022
@danieljbruce danieljbruce added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 20, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 20, 2022
@danieljbruce danieljbruce added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 21, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 21, 2022
@danieljbruce danieljbruce added the owlbot:run Add this label to trigger the Owlbot post processor. label Jun 21, 2022
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Jun 21, 2022
@danieljbruce danieljbruce added the owlbot:run Add this label to trigger the Owlbot post processor. label Jul 4, 2022
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Jul 4, 2022
@danieljbruce danieljbruce merged commit ecae5f3 into googleapis:main Jul 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: bigtable Issues related to the googleapis/nodejs-bigtable API. size: l Pull request size is large.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants