Skip to content
This repository was archived by the owner on May 27, 2025. It is now read-only.

Conversation

@ShookLyngs
Copy link
Collaborator

@ShookLyngs ShookLyngs commented Apr 29, 2024

Changes

  • Add context in the error object of the service lib
  • Add error handling recipe in service lib readme

Related issues

Details

You can now find context in the BtcAssetsApiError if the error was thrown from a request:

try { 
  ... 
} catch (e) {
  if (e instanceof BtcAssetsApiError) {
    console.log(e.context?.request);
    console.log(e.context?.response);
  }
}

This is the type of e.context:

interface BtcAssetsApiContext {
  request: {
    url: string;
    body?: Record<string, any>;
    params?: Record<string, any>;
  };
  response: {
    status: number;
    data?: Record<string, any> | string;
  };
}

@ShookLyngs ShookLyngs self-assigned this Apr 29, 2024
@ShookLyngs ShookLyngs changed the title feat(service): support throwing BtcAssetsApiError with context feat(service): throw service error with context Apr 29, 2024
@Flouse Flouse requested review from ahonn and duanyytop April 30, 2024 03:44
await this.init();
}

const packedParams = params ? '?' + new URLSearchParams(pickBy(params, (val) => val !== undefined)).toString() : '';
Copy link
Collaborator

@duanyytop duanyytop Apr 30, 2024

Choose a reason for hiding this comment

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

Is val => !!val more simple and clear?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think val !== undefined is better, if val is 0, it will be filtered out here

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes. The type of val is unknown

try {
return typeof body === 'string' ? JSON.parse(body) : void 0;
} catch {
return void 0;
Copy link
Contributor

@ahonn ahonn Apr 30, 2024

Choose a reason for hiding this comment

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

We should avoid using void 0, it is the same as undefined, but not as easy to understand.

Some libraries/packages use void 0 because undefined can be shadowed by local variables, and void 0 is shorter and can save a few bytes. You may see underscore or lodash used this way, but it doesn't mean we need it.
For modern JavaScript/TypeScript, this is a problem that does not require concern. Local variable shadowing cannot occur in TypeScript, and bundle size should be left to webpack/esbuild or other tools, rather than written as void 0

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree that undefined is better than void 0

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's kinda a coding style habit for me, I use it because it's easy to type.
I'll replace all void 0 to undefined in another pull request.

@ShookLyngs ShookLyngs requested a review from ahonn April 30, 2024 06:21
@Flouse Flouse mentioned this pull request Apr 30, 2024
4 tasks
@ShookLyngs ShookLyngs merged commit 9a10f8a into develop Apr 30, 2024
@duanyytop duanyytop deleted the feat/service-error-with-context branch May 10, 2024 12:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inject context into errors of rgbpp-sdk/service

5 participants