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.

Sign up for GitHub

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

Jump to bottom

GET value of a custom-variable via new HTTP API #2820

Closed
2 tasks done
LHasenbein opened this issue Mar 27, 2024 · 5 comments · Fixed by #2821
Closed
2 tasks done

GET value of a custom-variable via new HTTP API #2820

LHasenbein opened this issue Mar 27, 2024 · 5 comments · Fixed by #2821
Labels
area/backend Something in the core of companion BUG Something isn't working Reproducible This issue is reproducible. Multiple people are experiencing this.

Comments

@LHasenbein
Copy link

LHasenbein commented Mar 27, 2024 •
edited

Is this a bug in companion itself or a module?

  • I believe this to be a bug in companion

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

I have tested the Experimental 3.99.0+6806. I want to GET the value of a custom-variable via HTTP API like it's described in the doc. I've tried it with Javascript and php. I always get an html page back:

<!doctype html>

	<link href=\"/img/icons/256x256.png\" rel=\"shortcut icon\" />

	<link rel=\"manifest\" href=\"/manifest.json\" />
	<!-- PWA support -->
	<meta name=\"theme-color\" content=\"#000000\" />
	<!-- ios support -->
	<link rel=\"apple-touch-icon\" sizes=\"16x16\" href=\"/img/icons/16x16.png\" />
	<link rel=\"apple-touch-icon\" sizes=\"32x32\" href=\"/img/icons/32x32.png\" />
	<link rel=\"apple-touch-icon\" sizes=\"48x48\" href=\"/img/icons/48x48.png\" />
	<link rel=\"apple-touch-icon\" sizes=\"128x128\" href=\"/img/icons/128x128.png\" />
	<link rel=\"apple-touch-icon\" sizes=\"256x256\" href=\"/img/icons/256x256.png\" />

	<meta name=\"viewport\" content=\"width=device-width, initial-scale=1.0 , maximum-scale=1.0, user-scalable=0\" />
	<meta name=\"language\" content=\"en\" />
	<meta name=\"google\" content=\"notranslate\" />

	<meta http-equiv=\"X-UA-Compatible\" content=\"IE=edge\" />
	<meta http-equiv=\"Content-Language\" content=\"en\" />

	<meta name=\"theme-color\" content=\"#000000\" />
	<meta name=\"description\" content=\"Bitfocus Companion - Admin\" />
	<title>Bitfocus Companion - Admin</title>
	<script type=\"module\" crossorigin src=\"/assets/index-CbmXG7xe.js\"></script>
	<link rel=\"stylesheet\" crossorigin href=\"/assets/index-TlWH-Hzx.css\">
</head>

<body>
	<noscript>You need to enable JavaScript to run Companion.</noscript>
	<div id=\"root\"></div>
</body>

Maybe it's my fault. But I tried it as described in the documentation.

Steps To Reproduce

No response

Expected Behavior

No response

Environment (please complete the following information)

- OS: macOS 13.6.4
- Browser: Safari 17.3.1 / Chrome 123.0.6312.87
- Companion Version: 3.99.0+6806

Additional context

No response
@LHasenbein LHasenbein added the BUG Something isn't working label Mar 27, 2024
@dnmeid
Copy link
Member

dnmeid commented Mar 27, 2024

Please give the complete HTTP request you have used.

@LHasenbein
Copy link
Author

LHasenbein commented Mar 27, 2024 •
edited

@dnmeid

<script>
const url = 'http://localhost:8888/api/custom-variable/timecode/value';

fetch(url)
    .then(response => {
        if (!response.ok) {
            throw new Error('Netzwerkantwort war nicht ok');
        }
        return response.text();
    })
    .then(data => {
        console.log('Antwort:', data);
    })
    .catch(error => {
        console.error('Fehler beim Abrufen der Daten:', error);
    });
</script>

@dnmeid
Copy link
Member

dnmeid commented Mar 27, 2024

I can reproduce this bug.

@dnmeid
Copy link
Member

dnmeid commented Mar 28, 2024 •
edited

We have a default route which serves the home page if no other route is matched.

companion/companion/lib/UI/Express.js

Lines 139 to 142 in a0ad601

this.app.get('*', (req, res, next) => {
req.url = '/index.html'
return webuiServer(req, res, next)
})

The error seems to be that this route is matched before the the api route can match. I don't know why this is happening. All legacy api routes are matched before the default route and all http POST routes are matched before the default route.
The route in question is the only GET route for the new api.

@dnmeid dnmeid added Reproducible This issue is reproducible. Multiple people are experiencing this. area/backend Something in the core of companion labels Mar 28, 2024
@dnmeid
Copy link
Member

dnmeid commented Mar 28, 2024 •
edited

Ok, the reason is, that the new api routes are included here

companion/companion/lib/Service/Controller.js

Line 42 in a0ad601

this.httpApi.bindToApp(registry.ui.express.app)
after the routes of the web ui.
The default route of the web ui only handles get methods, so all post requests of the new api are fulfilled but the default get will match before any api get route can match.
@Julusian you did the way of binding the new api router. Is there any reason for that or can we change it to the same way the legacy api is included?

Another more hacky soution would be to not match the default route to *, but to /^(?!\/api).*/

dnmeid added a commit that referenced this issue Mar 29, 2024
@dnmeid
fix: routes precedence #2820
52a3557
@dnmeid dnmeid mentioned this issue Mar 29, 2024
Merged
@dnmeid dnmeid linked a pull request Mar 29, 2024 that will close this issue
fix: routes precedence #2820 #2821
Merged
@smudge1977 smudge1977 mentioned this issue May 4, 2024
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/backend Something in the core of companion BUG Something isn't working Reproducible This issue is reproducible. Multiple people are experiencing this.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: routes precedence #2820 bitfocus/companion
2 participants
@dnmeid @LHasenbein