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

[Feature Request] add NTFY support to enhance notifications options #90

Open
mgrimace opened this issue Apr 8, 2024 · 25 comments
Open
Labels
enhancement New feature or request

Comments

@mgrimace
Copy link
Contributor

mgrimace commented Apr 8, 2024

Updating FR for enhanced notifications

Apprise is a flexible notification service that allows for not only Discord notifications, but basically notifications to pretty well any service. This would mean that you wouldn't have to implement individual notification services for folks, and people could use their preferred notification method.

Use-case:

  • daily summary of points sent via NTFY, text, email, etc.
  • error notifications
  • 2FA (passwordless Support passwordless auth #129 ) number codes sent via preferred notification message. Useful for ‘headless’ and docker setups.
@mgrimace mgrimace changed the title [Feature Request] add apprise notifications [Feature Request] add apprise support to enhance notifications options Apr 8, 2024
@TheNetsky
Copy link
Owner

Feel free to make a PR with this implemented.

@TheNetsky TheNetsky added the enhancement New feature or request label Apr 22, 2024
@mgrimace
Copy link
Contributor Author

Feel free to make a PR with this implemented.

I've been trying to get it working, but no luck on my end. My goal is to add NTFY support, which is basically the same as discord webhook, but notifications can go directly to NTFY (web/app/etc) vs., Discord. It's here: https://www.npmjs.com/package/ntfy.

Apprise itself is a bigger job, but would enable notifications to any service, but personally my own use is just NTFY which should be easier.

@mgrimace mgrimace changed the title [Feature Request] add apprise support to enhance notifications options [Feature Request] add NTFY support to enhance notifications options Feb 2, 2025
@mgrimace
Copy link
Contributor Author

mgrimace commented Feb 2, 2025

I'm working on this FR now (since it's mine, just haven't had time to revisit in a while). This will let users optionally receive push notifications via NTFY (optionally) - https://github.com/binwiederhier/ntfy.

My ideal use of this is to push 2FA and passwordless codes out via notification. And, to send a summary however webhook currently sends info (log, error, etc.)

@mgrimace
Copy link
Contributor Author

mgrimace commented Feb 3, 2025

@TheNetsky Sorry for the ping, I don’t have Discord webhooks to compare and I have a (hopefully) quick question:
is the normal function of webhook notifications to output all of the items marked [LOG]?

With my basic NTFY support, I’m getting hundreds of notifications (each search generates a [LOG] item and is sent as an individual message, etc.). I don’t know if this is an issue with my own stuff, or if this is how it works with Discord Webhooks and wanted to check before I start debugging.

Update: starting a test branch with push notification support, notifications use NTFY with only output logs that match key words of 'completed' (e.g., all tasks completed), '2fa', and any warn or error. I'll have that up as a test branch shortly. Webhook support is untouched.

My goal with push notifications is:

  • a summary notification (script ran successfully
  • 2FA/passwordless code notification
  • error notification

ToDo:

  • Test a few runs
  • add starting/ending point values (starting is easy, ending is trickier at the moment)

@mgrimace
Copy link
Contributor Author

mgrimace commented Feb 4, 2025

Experimental support pushed to: https://github.com/mgrimace/Microsoft-Rewards-Script/tree/1.5

Working:

  • keyword-based push notifications are working (completed, 2FA)
  • Script functions 'normally' via docker (I didn't break anything as far as I can tell).

Next:

  • refine keywords
  • update readme
  • add points notifications

Please anyone is welcome and encouraged to test and suggestions/review requested.

Image

@mgrimace
Copy link
Contributor Author

mgrimace commented Feb 4, 2025

Oddly enough, whether I use completed or completed tasks for, I get many matches (e.g., bing search completed, etc.), but not
[2/3/2025, 8:40:20 PM] [PID: 197] [LOG] DESKTOP [MAIN-PRIMARY] Completed tasks for ALL accounts (which is the one I want!)

With the snippet from Logger.ts:

    // Send to NTFY only for specific logs
    if (type === 'warn' || type === 'error' || 
        message.toLowerCase().includes('completed tasks for') || 
        message.toLowerCase().includes('press the number') || 
        message.includes('2FA')) {
        Ntfy(cleanStr);
    }

@AariaX
Copy link
Contributor

AariaX commented Feb 4, 2025

Maybe the process exited first. Try awaiting ntfy

@mgrimace
Copy link
Contributor Author

mgrimace commented Feb 4, 2025

Maybe the process exited first. Try awaiting ntfy

Thanks, I’ll take a look at that. Since I am getting some ‘completed’ but not all (and all the warns/errors) I’m thinking it’s my rough Ntfy.ts. My guess is that I need to properly format the title/content and I might be hitting a character limit if it’s coming through as a ‘title’ only or something like that. Basically, I need to spend some time in the NTFY docs: https://docs.ntfy.sh/publish/#__tabbed_1_4

I am getting notifications though, which is a start!

@mgrimace
Copy link
Contributor Author

mgrimace commented Feb 5, 2025

Ok, I solved it! I was trying to use webhook like the existing discord functionality, but I swapped Ntfy.ts to use Curl instead and it's working much better and sending things. Fixed titles, body, priority, and emoji icons. I'll tidy up my debugging, and everything then push an update to my branch for further testing

@mgrimace
Copy link
Contributor Author

mgrimace commented Feb 5, 2025

update pushed to my test branch with everything working as expected now.
Ntfy.ts uses curl instead of webhook, which fixes unsent messages.

Logger.ts has a block added specific to NTFY. Since they're push notfications, we only want key notices:

  • Logs = 'completed tasks for', and 'press the number' which should cover summaries of script runs, and 2FA codes
  • Errors = 'ending' which should let us know if anything ended early
  • Warnings = 'aborting' and 'didn't gain' which should also let us know if anything didn't finish properly.

Other keyword based entries are welcome/encouraged.

Logger also sends the type (log, warn, or error) to Ntfy.ts, which prioritizes the notification (warn/error = high, log = default) and associates a particular emoji for the notification (e.g., alert emoji for errors).

Again, testing, review, contributions welcome/encouraged. I've added/removed a lot of debugging lines, and I'm pretty sure I pushed just the necessary changes but who knows.

@AariaX
Copy link
Contributor

AariaX commented Feb 5, 2025

Ok, I solved it! I was trying to use webhook like the existing discord functionality, but I swapped Ntfy.ts to use Curl instead and it's working much better and sending things. Fixed titles, body, priority, and emoji icons. I'll tidy up my debugging, and everything then push an update to my branch for further testing

I would try sticking with Axios, as not all systems—especially some Windows environments—have cURL installed, and Axios may be more efficient

@mgrimace
Copy link
Contributor Author

mgrimace commented Feb 5, 2025

Ok, I solved it! I was trying to use webhook like the existing discord functionality, but I swapped Ntfy.ts to use Curl instead and it's working much better and sending things. Fixed titles, body, priority, and emoji icons. I'll tidy up my debugging, and everything then push an update to my branch for further testing

I would try sticking with Axios, as not all systems—especially some Windows environments—have cURL installed, and Axios may be more efficient

Yeah, I just wasn’t able to get it working consistently using the existing axios webhook vs., a direct curl despite all my efforts. I don’t know enough about Axios, but if it’s possible to convert/send a curl style request through Axios (vs., webhook) that would be idea. If you have advice/suggestions how to do this, please take a look at my Ntfy.ts. Its above my skills, but I’ll work on it.

I was trying to piggyback on the current Axios/webhook, but for whatever reason some messages wouldn’t send while others would. The issue appears to be in formatting or structuring the message properly to send this way.

All of that to say, NTFY works well now with CURL. And, it doesn’t impact the functioning of the existing webhook/discord if people prefer that method + Axios for more detailed logs.

The benefit of NTFY is a quick self-hosted push notification, especially to receive 2FA codes and confirmation of runs, but it isn’t required in any way for the script to function (for me, just a quick text that says ‘it ran’ or ‘something didn’t finish, run it again’).

I’ll test for a bit, see what I can do about Axios - review/PRs welcome!

@mgrimace
Copy link
Contributor Author

mgrimace commented Feb 5, 2025

Here’s a quick example showing some search errors, using NTFY on an iOS lockscreen:

Image

@mgrimace
Copy link
Contributor Author

mgrimace commented Feb 6, 2025

Ok, testing an update to attempt to use Axios instead of curl for better support (e.g., windows).

Update: I cannot get it to send via axios vs., curl. I'll tidy up what I've got, add a note to the readme, and send over a PR for review. It's a definitely an option for folks, and it won't break any existing functionality (e.g., webhook as an alternative with more fulsome logs and no need for curl).

@AariaX
Copy link
Contributor

AariaX commented Feb 7, 2025

AariaX@7fc399f just simple await

IMG_20250207_144523.jpg

@mgrimace
Copy link
Contributor Author

mgrimace commented Feb 7, 2025

AariaX@7fc399f just simple await

You're amazing thanks so much! Your solution is much tidier, and I didn't know how to pull the pieces together myself to get it working.

What do you think of the key words? They could use some tuning - I get the same as your screenshot but for two accounts = 4 notifications (e.g., completed for 'all' and for 'email', which seems redundant). I worked through the logs and picked out things I thought would be useful, but could use some other eyes on it.

The icon/emoji can also be changed in Ntfy.sh - 'trophy' looks good for the [logs] as well.

Edit: adding keywords started tasks for account will be useful to notify when the script starts (that will notify one per account email)

@AariaX
Copy link
Contributor

AariaX commented Feb 7, 2025

AariaX@7fc399f just simple await

You're amazing thanks so much! Your solution is much tidier, and I didn't know how to pull the pieces together myself to get it working.

What do you think of the key words? They could use some tuning - I get the same as your screenshot but for two accounts = 4 notifications (e.g., completed for 'all' and for 'email', which seems redundant). I worked through the logs and picked out things I thought would be useful, but could use some other eyes on it.

The icon/emoji can also be changed in Ntfy.sh - 'trophy' looks good for the [logs] as well.

Edit: adding keywords started tasks for account will be useful to notify when the script starts (that will notify one per account email)

Yeah completed for all is not helpful with clusters unless we can actually get it to log correctly

@mgrimace
Copy link
Contributor Author

mgrimace commented Feb 7, 2025

Yeah completed for all is not helpful with clusters unless we can actually get it to log correctly

Yeah, the challenge has been getting the right keywords to filter everything marked [LOG]. An alternative might be to mark some entries (like started, 2fa, completed) as [STATUS] or something if possible, then just send the entire category as notifications instead of using keywords. That might help with localization/different languages perhaps?

How do you want to move forward with PRs and such? I plan to add some instructions to the readme and revise the keywords in the short term - would you like to use your fork as the base?

@TheNetsky
Copy link
Owner

I actually had a prototype for log filtering, which was a config option with the option to exclude functions.

example logsExcludeFunc: ["GET-TABS", "DAILY-CHECK-IN"]

@AariaX
Copy link
Contributor

AariaX commented Feb 7, 2025

Yeah completed for all is not helpful with clusters unless we can actually get it to log correctly

Yeah, the challenge has been getting the right keywords to filter everything marked [LOG]. An alternative might be to mark some entries (like started, 2fa, completed) as [STATUS] or something if possible, then just send the entire category as notifications instead of using keywords. That might help with localization/different languages perhaps?

How do you want to move forward with PRs and such? I plan to add some instructions to the readme and revise the keywords in the short term - would you like to use your fork as the base?

Feel free to go ahead with your fork. I don’t plan to dedicate much time to this.

@mgrimace
Copy link
Contributor Author

mgrimace commented Feb 7, 2025

Feel free to go ahead with your fork. I don’t plan to dedicate much time to this.

On it, testing your fixes, and some improvements for keywords then I'll push those changes to my test branch. Once I get some basic instructions in, I think it'll be ready for a PR back to main for review. Longer term goals would be that conditional logging, and generating notifications based on type vs keyword, but this should work fine in the short-term

Edit: using test branch: https://github.com/mgrimace/Microsoft-Rewards-Script/tree/ntfy

@mgrimace
Copy link
Contributor Author

mgrimace commented Feb 7, 2025

Added instructions
Tweaked icons
Tweaked notifications:

  • Script started for account email, per account
  • 2FA/passwordless codes
  • Script completed successfully for account email, per account (removed the redundant, script completed for ALL that was also per account)
  • Critical warnings
  • No error messages at the moment (they appear redundant with the warnings, such as if searches don't complete we don't need an error for each search as well).

@mgrimace
Copy link
Contributor Author

mgrimace commented Feb 7, 2025

Yeah completed for all is not helpful with clusters unless we can actually get it to log correctly

'Completed tasks for account email' was not showing up in my logs, I had to also add await log for that line as well.

           await log('main', 'MAIN-WORKER', `Completed tasks for account ${account.email}`, 'log', 'green')
        }

        await log(this.isMobile, 'MAIN-PRIMARY', 'Completed tasks for ALL accounts', 'log', 'green')
        process.exit()
    }

@mgrimace
Copy link
Contributor Author

mgrimace commented Feb 7, 2025

I do not get 'the script collected xxx points today' log item. Created a separate issue #226 so it's not buried here. I did add keyword 'the script collected' to Logger.ts so it can be sent as a push notification if the point summary can be fixed in the logs.

@mgrimace
Copy link
Contributor Author

For simplicity, I closed PR #227, and I'm working fresh from the new 1.5 release for NTFY support. I've added back in all the recent (working) changes to my branch: https://github.com/mgrimace/Microsoft-Rewards-Script/tree/ntfy

I'll double check that everything is still working as expected then re-open a tidier PR

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

No branches or pull requests

3 participants