Version stuff#558
Conversation
elias-ba
left a comment
elias-ba
left a comment
There was a problem hiding this comment.
Hey @josephjclark, had a look at this and it's really nice work. I've left one inline comment/question - totally up to you whether it's worth handling now or we just merge as is. Either way this is great stuff, well done and thanks! 🙌
| app.use(html()); | ||
|
|
||
| app.derive(() => ({ start: Date.now(), uuid: randomUUID() })); | ||
| app.onAfterHandle(({ set }) => { set.headers["X-Api-Version"] = pkg.version; }); |
There was a problem hiding this comment.
I think onAfterHandle only fires on the success path, so I'm not sure the header actually lands on all responses like the description says. Errors go through onError which doesn't set it, so wouldn't 401s from the auth hook and ApolloError/500s come back without the version? And for the streamed /services/*/stream responses, aren't the headers already flushed by the time this runs?
Not a blocker at all since the breaking-change guard rides on meta.apollo_version anyway. But should we set it somewhere that also covers the error/stream paths (mirror it in onError, or move it to onRequest/mapResponse)? And maybe add a test for an error response too, since the current one only checks the happy path?
There was a problem hiding this comment.
Great catch - thanks I'll take a close look at this
There was a problem hiding this comment.
So I've just added a 400 response from python and that includes the header
$ curl http://localhost:3000/services/echo -v -X POST
* Host localhost:3000 was resolved.
* IPv6: ::1
* IPv4: 127.0.0.1
* Trying [::1]:3000...
* Connected to localhost (::1) port 3000
> POST /services/echo HTTP/1.1
> Host: localhost:3000
> User-Agent: curl/8.5.0
> Accept: */*
>
< HTTP/1.1 400 Bad Request
< Content-Type: application/json
< X-Api-Version: 1.4.0
< Date: Fri, 26 Jun 2026 14:00:29 GMT
< Content-Length: 59
But in this case the typescript handler returns successfully. So if the handler threw an internal error for some reason we might not get the version number.
You make a good point about streaming though. I'm not actually sure where we'd return the version for a streaming API. I think I'll raise an issue for this.
One thing to note is that Lightning will make a HEAD call to the endpoint to get the version before it does anything. So the streaming thing is academic
|
Ach! Turns out the tag stuff doesn't work at all. I'm pulling this and we'll have to look at it later. |
* Version stuff (#558) * add server version header to all responses * return version key on chat payloads * add version to langfuse * add has_code_attachment tag to langfuse * changesets * update template * add an error to echo service for debugging * ensure services respond to HEAD requests * revert langfuse tags * Upgrade to Claude Opus (#553) * switch to opus * adjust tokens effort * add changeset * adjust tokens * version: 1.5.0 --------- Co-authored-by: Hanna Paasivirta <hanna@openfn.org>
2 participants
Short Description
Adds a bunch of tracking and versioning utilities to apollo
metakey; and on the langfuse clientFor traces with code attached, add ahas_code_attachmenttag in langfuseWhy?
AI Usage
Please disclose whether you've used AI in this work (it's cool, we just want to know!):
You can read more details in our Responsible AI Policy