fix: /api/v1/instance
#363
Conversation
…tials-endpoint Fix missing apps verify credentials endpoint
…oint Fix incompatible instance endpoint
[X] Use the official Mastodon default image for avatar
Change Default Images to Official Mastodon Avatar
|
@dario-piotrowicz Every playwright test from Is there another branch that I should be using as the base for this PR? cc @xtuc |
|
So, I reverted all the changes in this PR, updated from
|
|
Update: I fixed the problem by adjusting Playwright timeouts...and that surfaced an uncaught exception in my code, which I fixed. |
|
@DataDrivenMD I'm so sorry I didn't manage to reply earlier 🙇 Regarding SEO, it should work on main, is your branch up to date with main? and/or have you solved the issue? Regarding increasing the timeouts I really am not a big fan of that because the current timeouts were working just fine so if you need to increase them it would suggest that our code is getting slower or something like that (also they were longer at some point but it was getting annoying that we'd have to wait a while to see if they would fail so I did shorten the timeouts)
|
No worries! Thanks for closing the loop.
Prior to extending the timeouts I would see different e2e tests fail with every run (i.e. every time I called Once I extended the timeout period, the playwright tests defined in other files (i.e. not IOW: the e2e tests were working as designed but the short timeout period triggered some of them to fail prematurely in a flaky manner. This, in turn, made it impossible to figure out whether the tests were failing due a bug in my code vs. failing due to resource constraints vs. some other reason. Once the timeout period was extended, it was possible to see that a) there was an actual bug in the code and b) trace the exception. The timeout issue is fixed in PR #375, which has now been merged to PS - Sorry for the confusion, and I hope my answer clarifies the issue |
|
@xtuc Just want to flag that this PR is ready for re-review. I left two comments unresolved but added explanations for the proposed changes. |
Ah I see, ok I think I get it, you're talking about local runs right? maybe it just worked smoothly on my mac but on a different machine it could be not as reliable 🤔 Anyways thanks a bunch for the explanation 🙂 |
|
Turns out there were 2 issues causing e2e tests to fail (now fixed):
Separately, the TypeError that I shared above is still there but all tests are passing, so I don't know what to make of it. It seems to stem from the query builder code. |
|
Just upstreamed all the changes in this branch. Should be ready again for you @xtuc |
| const headers = { | ||
| ...cors(), | ||
| 'content-type': 'application/json; charset=utf-8', | ||
| } | ||
|
|
||
| const res: any = {} | ||
| if (env.ADMIN_EMAIL === '[email protected]') { | ||
| db = await getDatabase(env) |
There was a problem hiding this comment.
why do you need this? db is already passed in the function, do you still need to call getDatabase?
| // should go through the login flow and authenticate with Access. | ||
| // The documentation is incorrect and registrations is a boolean. | ||
| res.registrations = false | ||
| const statsDomain: string = env.ADMIN_EMAIL === '[email protected]' ? '0.0.0.0' : domain |
There was a problem hiding this comment.
I prefer to avoid test specific behaviors in prod endpoints
Taken together, this PR will improve compatibility with 3rd-party apps, including IceCubes and Ivory
/api/v1/instanceendpoint with Mastodon API spec: the bug was causing no admins to be returned under certain conditions; the lack of testing allowed this bug to fly under the radar until the value was called by the/api/v1/instancehandler/api/v1/instanceare accurateRelevant issues:
cc @xtuc @dario-piotrowicz: could either of you please take a look at this PR?