Return http error when the notification cannot be sent to the provider about apprise-api HOT 6 CLOSED

Sorry to take so long to get back to you.

I think your request seems okay. Would it maybe make more sense to send a 206 - Partial Content (see here)? Alternatively (follow the same link here) maybe 428 - Precondition Required if we use the 400 error codes?

What are your thoughts?

from apprise-api.

No worries, thanks for getting back to me!

I feel that returning a 2xx (indicating success from the server) would wrongly indicate the client that the message was sent to the provider. Moreover, from my understanding, 206 is used when requesting partial data, which doesn't seem to fit the message/notifications system in apprise.

About the 428 error code, I feel that returning a 4xx status (for client error) seems good. But for a Precondition error, I'm not sure that it's describing the same error that happens when a message cannot be sent to a provider (following a call to apprise's notify).

I'm thinking that the 400 - Bad request responds fits nicely when the client posts an invalid payload (such as my first example with the "invalid" urls).

But, would we be able, from apprise-api to check if the payload provided by the client is valid or not?

For the second one, we don't really know (as we only have a boolean returned by the notify call) if the request is malformed or if the provider is currently unable to process our request. Maybe, as apprise is more or less performing as a proxy, a 502 - bad gateway would be applicable.

What do you think about these ideas? I feel that the main point is to return a non-success status to clients when the notification cannot be sent.

from apprise-api.

I tend to overthink everything, so I'm actually thankful of your response and explanation 🙂

Unfortunately Apprise lacks the ability to capture the URLs that are invalid and report them correctly. So this will need some work in the backend of Apprise to support some kind of callback functionality... just a way of getting more details on how the parsing of the URLs went in such a way they can be re-reported to front facing systems.

Apprise also lacks the ability to tell you if the payload is valid or not because depending on what you have the overflow= directive set to, you technically never have a bad payload (see here). By default no one uses this variable so it's silently set to upstream, which means you won't know if the payload is bad until the upstream notification service processes it and tells you otherwise. But this would still invoke your suggested 400 - Bad request, so no big deal here.

But to just start off and mimic with how the CLI works, hence just to report a failure if not all of the notifications were sent correctly shouldn't be a problem. I agree with the 400 error code.

I'll try to get to this soon; I've been getting a lot of queued requests (even with Apprise itself) but due to some personal issues taking place in my life right now, I've been horrible at actioning any of them.

I do promise to look into this though. If you feel brave and feel like making your own Pull Request, I'll gladly accept that as well! 😉 If not, hopefully this weekend or next I can take a stab at it; it should be pretty straight forward.

Thoughts?

from apprise-api.

After looking at the code, 400 is an error code returned if you set bad options when posting the API (making it a real bad request). I didn't want to confuse the response, so i picked 424 as the error code instead. Failed Dependency documented as The request failed because it depended on another request and that request failed. It's an unofficial response code; but still used. I thought it fit nicely 🙂

I need to just update tests so i return to 100% test coverage; hopefully I'll get that done this week. Eitherway, there is a new branch now that will cover your issue.

from apprise-api.

Sorry for leaving you hanging, I was unable to respond the last few days.

The choice for 424 seems good to me, thanks for taking the time to implement this enhancement! Let me know if I can help in any way.

Best regards,

from apprise-api.

I just merged the changes into the master branch. You should be able to update your docker container and get the new code that will error as you requested in the event one or more services fail to be notified. Thanks for raising this ticket!

I'm going to close it now, but please feel free to re-open it and/or open another if you have any more great ideas! 🙂

from apprise-api.