-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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
proposal: x/tools/internal/jsonrpc2_v2: export wireError #64882
Labels
NeedsInvestigation
Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Comments
This doesn't need a proposal as it doesn't change the public API. Feel free to send us a CL and we can discuss it there. |
ianlancetaylor
added
NeedsInvestigation
Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
and removed
Proposal
labels
Feb 7, 2024
Change https://go.dev/cl/562515 mentions this issue: |
Change https://go.dev/cl/562575 mentions this issue: |
Change https://go.dev/cl/562575 mentions this issue: |
gopherbot
pushed a commit
to golang/tools
that referenced
this issue
Feb 14, 2024
This makes it possible for users of the package to us the optional error data field in error construction and error handling logic. Updates golang/go#64882 Change-Id: Iaa554f42ff42a0b7355605ba0b49ac67f623da15 Reviewed-on: https://go-review.googlesource.com/c/tools/+/562575 Reviewed-by: Alan Donovan <[email protected]> Reviewed-by: Than McIntosh <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
NeedsInvestigation
Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Proposal Details
This may not require a proposal since the package is internal. If a formal proposal is needed, I will flesh this out.
Problem
Currently the
jsonrpc2.wireError
does not provide access to theData
field within a handler. This makes it impossible for a handler to add data to a JSON RPC2.0 error in a way that will result in the caller receiving it due to the way that errors are handled by the response marshalling code path.Proposal
I propose that the
wireError
type be exported. This will enable handlers to add values to theData
field and callers to be able to inspect the returned error data.A less appealing alternative would be to add a new error creation function that adds the data field (additional API function surface) or alter the existing
NewError
(a relatively invasive and backward incompatible change, though note that the package is internal). Both of these changes would also require the addition an accessor for callers to be able to inspect theData
field.Compatibility
The proposal is fully backward compatible because it exposes a previously unexported type. The package is currently internal.
Cost/Benefit
The package is currently internal and no internal user appears to have a need for this functionality. I make use of the package as a mechanically extracted non-internal package. If the proposal is rejected, I can work around this by making the change in the extracted package, but I'd like to see whether this is needed before I make that change.
The text was updated successfully, but these errors were encountered: