-
Notifications
You must be signed in to change notification settings - Fork 319
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
WebHooks aren't being processed? #1730
Comments
* Applies to OpenUserJS/OpenUserJS.org#1730
We do not. We return the http status codes that shows up in your webhook logs on GH only. Browsing through the
Well as you know it probably isn't. I'll look at the PR in a bit but I'll probably have to try that VPS side directly since dev environment doesn't even process the webhook. If @sizzlemctwizzle would be available to review #1729 that would be appreciated.
I see a date updated difference on one of those. Although I just did my usual "Hello, World" test push and it sync'd well. You could be running into GH throttling with #1705 but unsure atm. |
Instances: 1. Author has webhook enabled and GH sends update notice before imported on OUJS. 2. Author deleted OUJS script and still receiving webhook notice. There's still the extremely rare chance that this could be a 500, if *mongoose*,*mongodb*, and S3 has a catastrophic connection error. Since the bulk of this is usually client error denoting as such in case GH is paying attention and possibly doing something on their end by killing further notices. If true then this part "could be" related to OpenUserJS#1730 Post OpenUserJS#1731
Instances: 1. Author has webhook enabled and GH sends update notice before imported on OUJS. 2. Author deleted OUJS script and still receiving webhook notice. There's still the extremely rare chance that this could be a 500, if *mongoose*,*mongodb*, and S3 has a catastrophic connection error. Since the bulk of this is usually client error denoting as such in case GH is paying attention and possibly doing something on their end by killing further notices. If true then this part "could be" related to #1730 Post #1731 Auto-merge
Thanks, I didn't realise those existed! I can report that I have been receiving 202s back from all the hook calls. (Both before and after your recent changes.) I'm also happy to report that one update I made on 17th August did reach the website: However other scripts have not been updated, including this one which I updated just now (at Although the webhooks logs show I did get a 202 back from OUJS. (From GitHub's request with Please let me know if there is anything I can do to help. Cheers. |
* This is used for Authors to track the internal processing status of each script. * Only shows up per Author i.e. nobody else and you *must* be logged in to view * Search might be incorrect atm... still tinkering with that part. * Hide some pre-existing GH items if no auth strategy present. * Eventually more is needed but this is a start. Applies to OpenUserJS#1730 and a few related others in the past; Will eventually affect OpenUserJS#430 by moving debugging to Author page instead of OUJS intervention.
* This is used for Authors to track the internal processing status of each script. * Only shows up per Author i.e. nobody else and you *must* be logged in to view * Search might be incorrect atm... still tinkering with that part. * Hide some pre-existing GH items if no auth strategy present. * Eventually more is needed but this is a start. Applies to #1730 and a few related others in the past; Will eventually affect #430 by moving debugging to Author page instead of OUJS intervention. Auto-merge
When you next update on GH try a visit to your profile page, while logged into OUJS, and check the Syncs menu item in the profile nav bar and let me know if there are any issues. This hasn't been fully tested but mostly tested and should be a good start for you. |
I was watching as you did this.... Your script has a The SPDX codes changed at some point to be one of these:
That's why you are getting a failure on the script you tried to sync a few moments ago. |
Great work Marti, thanks a lot for this. I can report that the message is helpfully appearing on the
That makes it much easier to debug. The script in question is now updated! And I will proceed to fix the various other script(s) at my end. (I'm not sure why |
A small suggestion: Reverse the order of the log messages, so most recent is at the top. That's how GitHub presents its hooks logs (it's common to present notifications in general), and it will make it "easier" to see the latest log when there are many in the list. |
That is the default here. |
Ah I see. It's different depending which tab I choose. 👍 |
Hope this helps diagnose things when I'm not here... I still have a lot to do in these areas but had a few hours to throw this together since it's a common question. Glad you like it. :) |
* Would have thought this would need to be in partial but ehh Post OpenUserJS#1730
* Would have thought this would need to be in partial but ehh Post #1730 Auto-merge and brake time.
* We really don't need to keep this info around forever. * Indicate in nomenclature Applies to OpenUserJS#1730
* We really don't need to keep this info around forever. * Indicate in nomenclature Applies to #1730 Auto-merge
* Possible recommendation as `expires` alias to `expireAfterSeconds` may need this to properly set values of an index that is created app side to sync with back-end. * Increase event `emitter.setMaxListeners` to accommodate additional MongoDB usage with error thrown: ``` console Index event triggered/trapped for User model Index event triggered/trapped for Discussion model Index event triggered/trapped for Strategy model Index event triggered/trapped for Remove model Index event triggered/trapped for Comment model Index event triggered/trapped for Group model Index event triggered/trapped for Flag model Index event triggered/trapped for Vote model (node:10567) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 open listeners added to [NativeConnection]. Use emitter.setMaxListeners() to increase limit ``` Applies to #1744 #1730 and followup for #1516 ... followup needed on mLab for index that's already there *(which I didn't put there)* Auto-merge
* Additional confirmation is needed when GH contacts us with a predefined secret * New dep... synchronous git-rev *(forked by another)* ... needed because some of our code is too fast for the asynchronous return dep. * Add missing UA in one request for the GH client ... pinging the certificate doesn't really need one but symmetry. Applies to OpenUserJS#1730
* Additional confirmation is needed when GH contacts us with a predefined secret * New dep... synchronous git-rev *(forked by another)* ... needed because some of our code is too fast for the asynchronous return dep. * Add missing UA in one request for the GH client ... pinging the certificate doesn't really need one but symmetry. Applies to #1730 Auto-merge
* Convert to storing a String instead Post OpenUserJS#1730
* Convert to storing a String instead Post #1730 Auto-merge
* Better error handling for Write Script Online. * Better error handling for Upload Script. Fixes potential server trips and/or hangings. * Repo URL change detected on GH for *formidable*... updated README.md linkage. * Some symmetry in these areas. Post OpenUserJS#1730 OpenUserJS#430 OpenUserJS#37
* Catch GH import routine * Add `#bypass` to statusCodePage Post OpenUserJS#1731 , applies to OpenUserJS#430 OpenUserJS#37 and closes OpenUserJS#1730
Recently on my Syncs page I am getting a lot of "Updating but no script found." I think some years ago GM for Firefox might have lowercased all my script names, and I went along with it. [Edit: Never mind. This was not the reason.] I can rename the file, but this is a bit of a hassle (and breaks some of my own processes). Would it be possible to make the (And maybe fall back to a case-sensitive search, if multiple case-insensitive matches were found.) [Edit: Forget this suggestion. As Marti explains, that wasn't the problem.] Update: This was happening with iMDB_Large_Images.user.js but I have now deleted that, and imported imdb_large_images.user.js |
Nope. I'm the one that was instructed to make it case sensitive and it should always be. See #656 (comment) and forward in that issue. (lots of links in there too) Picking on one of your sync's at https://raw.githubusercontent.com/joeytwiddle/code/master/other/gm_scripts/wikimedia/wikimedia.user.js#bypass=true ... that isn't on OUJS with the
etc.... Really don't want to go through all 40 of your scripts along with syncs to validate by hand... how about you do that and post back and I'll check those. :) As you may have noticed we still get the update request notice if you don't publish a script to OUJS from GH's webhook... so keep that in mind. Those particular 404's you can ignore if you have not published to OUJS. However the GH issue one could be them... could be you... as long as the |
Good point with Wikimedia +. I had renamed it so it better described what it actually did. Thanks for clarifying. So it looks like the problem is that I recently added "[fork]" and "[adopted]" to the names of many of my scripts, to clearly distinguish which are original work and which are not. (When doing this, I did not change the filenames, just the Is there any way to smoothly rename a script on OUJS? There is a thread here where you suggested:
but that is a little more cumbersome than I had hoped. FWIW, Freaky Gorse had no problem with my renames. It must use some different logic for linking to upstream. I'm not sure if it's worth considering a different approach for OUJS. My naive assumption was that the filename / URL would be the unique identifier, rather than the name in the metadata. Anyway, making such a change may not be easy, even if it is desirable. I did try editing the Now I should consider whether to un-rename all my scripts back to how they were before, or duplicate them and deprecate the older copies, in line with your forum suggestion. Or hack away at OUJS source code. 😉 |
That's because you are just a number over there ... https://greasyfork.org/en/users/8615-you_are_just_a_number_over_there
It's not and sizzle won't agree with it either from past historical posts.
Your choice... but you could have modified
It is smooth. Grr. |
I often push updates to my userscripts, but it seems some of the new versions haven't been loaded from github for some time, whilst others have: https://openuserjs.org/users/joeytwiddle/scripts
For example, this one has changed a few times (see its History): https://github.com/joeytwiddle/code/blob/master/other/gm_scripts/Related_Links_Pager/Related_Links_Pager.user.js
Do you have some debugging information about which part of the hooks process has a problem?
The script mentioned above is 50KB, well within the 1MB limit specific in
settings.json
(at least in the dev settings).I see the hooks process starts at
exports.webhook
inscriptStorage.js
and goes on to callrepoManager.prototype.loadScripts
inrepoManager.js
.I would assume the
fetchRaw()
is working as it is, but if you want me to add authentication to that, I could do that.I just did a push to GitHub at
new Date(1596957303135)
so perhaps you might see something logged around that time.https://openuserjs.org/scripts/joeytwiddle/Wikipedia_Inline_Article_Viewer
https://github.com/joeytwiddle/code/blob/master/other/gm_scripts/wikipedia_inline_article/wikipedia_inline_article.user.js
The text was updated successfully, but these errors were encountered: