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

Ship a single shared instance of TwirpError rather than per-service #172

Open
1 task done
noahseger opened this issue Jul 31, 2024 · 3 comments
Open
1 task done

Comments

@noahseger
Copy link

Preflight Checklist

  • I have searched the issue tracker for an issue that matches the one I want to file, without success.

Problem Description

We have a lot of service definitions, and each one generates a separate TwirpError class. It would be nice to have a single instance.

Benefits:

  • Allows generic middleware to throw TwirpErrors without relying on any individual service
  • Less PHP code to load

Proposed Solution

We could make this change in a backwards-compatible way by including a new protoc option to disable generating TwirpError and instead import it from twirphp directly.

Alternatives Considered

No response

Additional Information

No response

@sagikazarmark
Copy link
Member

@noahseger thanks for opening an issue.

Allows generic middleware to throw TwirpErrors without relying on any individual service

You can already use the Twirp\Error interface to handle errors.

We could make this change in a backwards-compatible way by including a new protoc option to disable generating TwirpError and instead import it from twirphp directly.

Sounds good to me. Would you be willing to provide a PR?

@noahseger
Copy link
Author

@sagikazarmark thanks so much for validating — yes, of course we can contribute a PR :) It might be a few weeks but will be a big help if you can review.

@sagikazarmark
Copy link
Member

No rush! Happy to review early drafts and provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants