Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prefix error messages with "[@octokit/webhooks] " #325

Closed
oscard0m opened this issue Oct 13, 2020 · 3 comments · Fixed by #339
Closed

Prefix error messages with "[@octokit/webhooks] " #325

oscard0m opened this issue Oct 13, 2020 · 3 comments · Fixed by #339
Assignees
Labels
Status: Up for grabs Issues that are ready to be worked on by anyone Type: Maintenance Any dependency, housekeeping, and clean up Issue or PR

Comments

@oscard0m
Copy link
Member

To add initial [@octokit/webhooks] starting message on every Error of the project.


I'd prefer to control it. GitHub only supports sha1 and sha256, it makes no sense for us to support any other algorithm, and could be the source of a hard to debug problem. Throwing an error will make it obvious.

As a side note, I started to prefix error and log messages with the package name

  if (!Object.values(Algorithm).includes(algorithm as Algorithm)) {
    throw new TypeError("[@octokit/webhooks] algorithm should be 'sha1' or 'sha256'");
  }

We should do that in with all errors / log messages, but in a separate PR

Originally posted by @gr2m in #322 (comment)

@oscard0m oscard0m added the Type: Maintenance Any dependency, housekeeping, and clean up Issue or PR label Oct 13, 2020
@gr2m gr2m changed the title To add scope (initial prefix message) when throwing errors on this project Prefix error messages with "[@octokit/webhooks] " Oct 13, 2020
@gr2m gr2m added good first issue Status: Up for grabs Issues that are ready to be worked on by anyone labels Oct 14, 2020
@gr2m
Copy link
Contributor

gr2m commented Oct 14, 2020

Feel free to work on it if you like, but I think this would also make a great issue for a new contributor to pick up.

Instructions:

  1. Comment below if you want to work on it
  2. Search the code for "new Error" and "new TypeError"
  3. Prefix the messages with "[@octokit/webhooks] ", e.g.
- throw new Error("options.secret required");
+ throw new Error("[@octokit/webhooks] options.secret required");
  1. Update tests as needed
  2. Create a pull request

@srujandeshpande
Copy link
Contributor

I'd like to work on this!

@github-actions
Copy link
Contributor

🎉 This issue has been resolved in version 7.15.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Up for grabs Issues that are ready to be worked on by anyone Type: Maintenance Any dependency, housekeeping, and clean up Issue or PR
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants