feat: added verbose logging to errors

[rkoval]

Please describe the changes this PR makes and why it should be merged:

Status

• [x] Code changes have been tested.
  • I was unable to run the tests locally. It seems like they're commented out in CI, so I'm unsure how to verify the behavior I created. Please advise!! (edit: I spoke with @zaida04 about this and he's going to look into this)

Semantic versioning classification:

• [x] This PR changes the library's interface (methods or parameters added)
• [x] This PR includes breaking changes (methods removed or renamed, parameters moved or removed)
• [ ] This PR only includes non-code changes, like changes to documentation, README, etc.

---

Yo! Ryan from Guilded Staff here! :pizza:

First of all, thank you so much for your continued interest in Guilded and its bot API!!! We love seeing the passion in the community for building fun stuff against the product. This library has opened the door to JavaScript and TypeScript devs to be able to write bots quickly in these languages. Nice work there!!

This PR serves to bring this library to conformance with the "Verbose Logging" guildeline in Guilded's Bot Library Guidelines document. We have been having trouble working through various support issues recently in our community server for this library, and I think a lot of the pain points come from missing some details from the "Verbose Logging" [1]. From the brief look I took, it does look like this library conforms to the other guidelines in that doc though, so nice work otherwise!!

Some outstanding issues with this PR that I will hand off to the library maintainers to fix (mostly done in the interest of time, sorry for wall of text!):

• There is a lot of wonkiness in the error constructor because I was trying to appease backwards compatibility of the exported errors. I am not a TypeScript expert, so it's entirely possible this can be done in a better way [2]. I'd encourage you to heavily scrutinize what I wrote to ensure I didn't leave anything through the cracks. I'd also encourage you to create a deprecation path for the previous approach so that this wonkiness can be eventually removed if not done during this code review.
• I did not obfuscate the authorization header due because I don't know how this library handles case sensitivity of the header given the underlying object is a POJO, not a Headers object. This must be done before merging, as it's a security risk outputting it as-is. There are loud FIXMEs explaining where
• I did not spend time properly typing everything. I'll leave that up to the library devs though because there may be context I'm missing as to how to properly type things in accordance to patterns that exist in the rest of the library. I'd encourage you to fill the gaps that I left (e.g., not using any, etc.)!
• I am not sure how best to document this new behavior, as the constructor may be more of an internally used mechanism and the errors are only exported for instanceof checks. It's possible nothing is needed here?
• There may be miscellaneous other things that don't conform to patterns that exist elsewhere in the codebase. Please feel free to blow away and rework anything I did here as long as the end state is that the requests and responses are being output as I've originally done here.

I encourage you to prioritize fixing the outstanding issues, merging the PR, and creating a new release so that the Guilded support team can better help debugging various API issues going forward. Please let me know how I can help in facilitating a quick turnaround here.

---

• [1] It's entirely possible that the existing verbiage in our "Verbose Logging" guideline is not clear enough for what libraries should look like. Please let us know if that is the case so that we can ensure other bot/library devs can understand this critical point!
• [2] A question I had: can you type entire function arg arrays so that the V1 and V2 ways can be mutually exclusive? This would potentially make it so the compiler enforces that the constructors can only be invoked in 2 specific ways: the v2 way in (msg: string, methodOrRequest: RequestOptions, pathOrResponse: ResponseDetails) or v1 way in (msg: string, methodOrRequest: string, pathOrResponse: string, code: number). This is in contrast with what is currently written that technically allows for mixing and matching of parameters such that you could invoke it like (msg: string, methodOrRequest: string, pathOrResponse: ResponseDetails), which is somewhere in between V1 and V2 usage and is bad.

[changeset-bot[bot]]

🦋 Changeset detected

Latest commit: ba7c9602b335f2f330a74f73fa5c0942a40cee9e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages

| Name | Type | | ------------------------- | ----- | | @guildedjs/rest | Major | | @guildedjs/proxy-service | Minor | | guilded.js | Patch | | @guildedjs/webhook-client | Patch | | @guildedjs/gil | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[ItzNxthaniel]

Thank you for your contribution fellow pizza man! 🍕

[zaida04]

Hey! Thank you for the contribution, we really appreciate it :D

I did some changes on your PR for formatting/type narrowing to bring it up to merge standard. It is pending testing with our examples repo to ensure it stays consistent in behavior.

I have also done the following:

• Added obfuscation on the auth header for error objects
• Removed backwards compatibility for the old way of instantiating GuildedAPIError and PermissionsError in favor of having @guildedjs/rest release on a semver major increment.

In regards to [2] in your footnotes: if we wanted to keep the same functionality but have different signatures so that there isn't incompatible type mix and matching, we could use method overloaders to enforce different, exclusive signatures.

Don't worry about any documentation regarding your changes either, as our TSDoc generator automatically creates new docs files for parsing/hosting on every commit/merge to main.

And a response to [1] as well, the guidelines are clear enough for us to understand. We had actually attempted in the past to conform to the logging standard set in that document, but ran into an issue as we wanted the actual Response object to also be passed as an object whole into an event on the .emitter property of the RestManager so that users could have opt-in logging on any REST request, regardless of failure or success. However, we found out it wasn't doable without consuming/cloning the Response object which was not advisable due to thread lockups. As such, we abandoned the attempt and settled on trying again later at some point before the publicization of the API (which we didn't hold ourselves to).

[rkoval]

oooh the method overloaders feature of TS is interesting. i think this would've been what i would've wanted had we kept the wonkiness i wrote :). thanks for the tip!

i think the other changes here make sense. thank you for cleaning this up! can you please post a screenshot though of the output from an error just so i can verify what it looks like?

[zaida04]

(For record keeping sake, I'll note in this PR that screenshots of the error outputs were sent privately)

[zaida04]

Thank you for the contribution! 🚀