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

Generic endpoint and content type default #363

Open
qdm12 opened this issue Jun 12, 2023 · 5 comments
Open

Generic endpoint and content type default #363

qdm12 opened this issue Jun 12, 2023 · 5 comments

Comments

@qdm12
Copy link

qdm12 commented Jun 12, 2023

Hi there,

For the generic endpoint, it seems the content type defaults to application/json.
Now the problem is other endpoints (i.e. gotify) will json encode their params map, so if the user calls Send on a *router.ServiceRouter with a json encoded string, it will get json encoded twice.
Two questions from this:

  1. Shouldn't the generic endpoint content type default to text/plain instead of application/json so it works nicely with other services routed in the router?
  2. Is it possible to specify content type per service when passing a types.Params into a router Send method?

Thanks!

@piksel
Copy link
Member

piksel commented Jun 13, 2023

Hello. Yeah, the generic endpoint is a bit odd in that it mainly serves as a last resort for getting notifications working for obscure (or just unsupported) scenarios... the default being application/json is just because that has been the most common use case for it, but it requires a customized body. I should also point out that you can add ?template=json to have it encode the payload params as a json string.

@qdm12
Copy link
Author

qdm12 commented Jun 13, 2023

Great, thanks for the feedback, being able to specify the content type by address solves it 👍 Shameful admission I didn't check and just trusted my users over at ddns-updater that there was no such option 😄

Now just over-thinking, since you're still on v0.x.x releases, should we change the default generic content type to text/plain? 🤔 Especially given the Go API, it was not obvious at all I would say.

@ThinkChaos
Copy link

I agree making the default text/plain would make more sense. template=json could imply application/json.

@piksel
Copy link
Member

piksel commented Jun 26, 2023

We could change it whenever 1.0 rolls out, but I hesitate to make any breaking changes since shoutrrr is mostly a "configure and forget" type of thing.

@qdm12
Copy link
Author

qdm12 commented Jun 26, 2023

We could change it whenever 1.0 rolls out

I think it would be better to have on an unstable release such as v0.8.0.

On the other hand, could we maybe have the content type as adaptive depending on the message? For example if it looks like json (try decoding it or use a regex), have it as application/json, otherwise default to text/plain. That shouldn't break any usage, and would be a nice additional feature. It would hurt performance a bit, but I really don't think this is important in this situation.

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

3 participants