From 6358457ec8b17bd81fa018b448e533c9ab84e054 Mon Sep 17 00:00:00 2001 From: CahidArda Date: Thu, 5 Dec 2024 19:58:01 +0300 Subject: [PATCH 1/2] fix: resend integration headers content-type header wasn't being sent, so resend couldn't process the request. But the integration worked before. So I am suspecting that QStash used to set a default content header but it doesn't anymore. To fix the issue, I added a utility to properly merge the headers and added tests to make sure that we don't miss this behavior. --- src/client/api/email.test.ts | 13 ++++++++++ src/client/api/utils.ts | 48 +++++++++++++++++++++++++++--------- src/client/client.ts | 9 +++---- src/client/queue.ts | 3 +-- 4 files changed, 55 insertions(+), 18 deletions(-) diff --git a/src/client/api/email.test.ts b/src/client/api/email.test.ts index 204cfbb1..9e1de425 100644 --- a/src/client/api/email.test.ts +++ b/src/client/api/email.test.ts @@ -9,6 +9,9 @@ describe("email", () => { const resendToken = nanoid(); const client = new Client({ baseUrl: MOCK_QSTASH_SERVER_URL, token: qstashToken }); + const header = "my-header"; + const headerValue = "my-header-value"; + test("should use resend", async () => { await mockQStashServer({ execute: async () => { @@ -17,6 +20,9 @@ describe("email", () => { name: "email", provider: resend({ token: resendToken }), }, + headers: { + [header]: headerValue, + }, body: { from: "Acme ", to: ["delivered@resend.dev"], @@ -42,6 +48,8 @@ describe("email", () => { headers: { authorization: `Bearer ${qstashToken}`, "upstash-forward-authorization": `Bearer ${resendToken}`, + "content-type": "application/json", + [`upstash-forward-${header}`]: headerValue, }, }, }); @@ -55,6 +63,9 @@ describe("email", () => { name: "email", provider: resend({ token: resendToken, batch: true }), }, + headers: { + [header]: headerValue, + }, body: [ { from: "Acme ", @@ -96,6 +107,8 @@ describe("email", () => { headers: { authorization: `Bearer ${qstashToken}`, "upstash-forward-authorization": `Bearer ${resendToken}`, + "content-type": "application/json", + [`upstash-forward-${header}`]: headerValue, }, }, }); diff --git a/src/client/api/utils.ts b/src/client/api/utils.ts index 3342382e..58385e84 100644 --- a/src/client/api/utils.ts +++ b/src/client/api/utils.ts @@ -1,6 +1,7 @@ import type { PublishRequest } from "../client"; import type { LLMOptions, ProviderInfo, PublishEmailApi, PublishLLMApi } from "./types"; import { upstash } from "./llm"; +import type { HeadersInit } from "../types"; /** * copies and updates the request by removing the api field and adding url & headers. @@ -42,20 +43,53 @@ export const getProviderInfo = ( return finalProvider.onFinish(providerInfo, parameters); }; +/** + * joins two header sets. If the same header exists in both headers and record, + * one in headers is used. + * + * The reason why we added this method is because the following doesn't work: + * + * ```ts + * const joined = { + * ...headers, + * ...record + * } + * ``` + * + * `headers.toJSON` could have worked, but it exists in bun, and not necessarily in + * other runtimes. + * + * @param headers Headers object + * @param record record + * @returns joined header + */ +const safeJoinHeaders = (headers: Headers, record: Record) => { + const joinedHeaders = new Headers(record); + for (const [header, value] of headers.entries()) { + joinedHeaders.set(header, value); + } + return joinedHeaders as HeadersInit; +}; + /** * copies and updates the request by removing the api field and adding url & headers. * - * if there is no api field, simply returns. + * if there is no api field, simply returns after overwriting headers with the passed headers. * * @param request request with api field + * @param headers processed headers. Previously, these headers were assigned to the request + * when the headers were calculated. But PublishRequest.request type (HeadersInit) is broader + * than headers (Headers). PublishRequest.request is harder to work with, so we set them here. * @param upstashToken used if provider is upstash and token is not set * @returns updated request */ export const processApi = ( request: PublishRequest, + headers: Headers, upstashToken: string ): PublishRequest => { if (!request.api) { + request.headers = headers; return request; } @@ -69,11 +103,7 @@ export const processApi = ( return { ...request, - // @ts-expect-error undici header conflict - headers: new Headers({ - ...request.headers, - ...appendHeaders, - }), + headers: safeJoinHeaders(headers, appendHeaders), ...(owner === "upstash" && !request.api.analytics ? { api: { name: "llm" }, url: undefined, callback } : { url, api: undefined }), @@ -81,11 +111,7 @@ export const processApi = ( } else { return { ...request, - // @ts-expect-error undici header conflict - headers: new Headers({ - ...request.headers, - ...appendHeaders, - }), + headers: safeJoinHeaders(headers, appendHeaders), url, api: undefined, }; diff --git a/src/client/client.ts b/src/client/client.ts index dba6ebfa..a04fc6af 100644 --- a/src/client/client.ts +++ b/src/client/client.ts @@ -378,11 +378,10 @@ export class Client { //@ts-expect-error caused by undici and bunjs type overlap const headers = prefixHeaders(new Headers(request.headers)); headers.set("Content-Type", "application/json"); - request.headers = headers; //@ts-expect-error hacky way to get bearer token const upstashToken = String(this.http.authorization).split("Bearer ")[1]; - const nonApiRequest = processApi(request, upstashToken); + const nonApiRequest = processApi(request, headers, upstashToken); // @ts-expect-error it's just internal const response = await this.publish({ @@ -436,12 +435,12 @@ export class Client { if ("body" in message) { message.body = JSON.stringify(message.body) as unknown as TBody; } - //@ts-expect-error caused by undici and bunjs type overlap - message.headers = new Headers(message.headers); //@ts-expect-error hacky way to get bearer token const upstashToken = String(this.http.authorization).split("Bearer ")[1]; - const nonApiMessage = processApi(message, upstashToken); + + //@ts-expect-error caused by undici and bunjs type overlap + const nonApiMessage = processApi(message, new Headers(message.headers), upstashToken); (nonApiMessage.headers as Headers).set("Content-Type", "application/json"); diff --git a/src/client/queue.ts b/src/client/queue.ts index 7cf26aeb..53c4e193 100644 --- a/src/client/queue.ts +++ b/src/client/queue.ts @@ -134,11 +134,10 @@ export class Queue { //@ts-expect-error caused by undici and bunjs type overlap const headers = prefixHeaders(new Headers(request.headers)); headers.set("Content-Type", "application/json"); - request.headers = headers; //@ts-expect-error hacky way to get bearer token const upstashToken = String(this.http.authorization).split("Bearer ")[1]; - const nonApiRequest = processApi(request, upstashToken); + const nonApiRequest = processApi(request, headers, upstashToken); const response = await this.enqueue({ ...nonApiRequest, From 59c0bdbc5ca9fddb570673808f38afbbfa50c603 Mon Sep 17 00:00:00 2001 From: CahidArda Date: Fri, 6 Dec 2024 13:45:55 +0300 Subject: [PATCH 2/2] fix: add method option to providers --- src/client/api/base.ts | 2 ++ src/client/api/email.test.ts | 63 ++++++++++++++++++++++++++++++++++++ src/client/api/email.ts | 1 + src/client/api/llm.ts | 1 + src/client/api/types.ts | 5 +++ src/client/api/utils.ts | 5 ++- 6 files changed, 76 insertions(+), 1 deletion(-) diff --git a/src/client/api/base.ts b/src/client/api/base.ts index 1ed95166..caf4fb49 100644 --- a/src/client/api/base.ts +++ b/src/client/api/base.ts @@ -1,7 +1,9 @@ +import type { HTTPMethods } from "../types"; import type { ApiKind, Owner, ProviderInfo } from "./types"; export abstract class BaseProvider { public abstract readonly apiKind: TName; + public abstract readonly method: HTTPMethods; public readonly baseUrl: string; public token: string; diff --git a/src/client/api/email.test.ts b/src/client/api/email.test.ts index 9e1de425..07b563bf 100644 --- a/src/client/api/email.test.ts +++ b/src/client/api/email.test.ts @@ -50,6 +50,7 @@ describe("email", () => { "upstash-forward-authorization": `Bearer ${resendToken}`, "content-type": "application/json", [`upstash-forward-${header}`]: headerValue, + "upstash-method": "POST", }, }, }); @@ -109,6 +110,68 @@ describe("email", () => { "upstash-forward-authorization": `Bearer ${resendToken}`, "content-type": "application/json", [`upstash-forward-${header}`]: headerValue, + "upstash-method": "POST", + }, + }, + }); + }); + + test("should be able to overwrite method", async () => { + await mockQStashServer({ + execute: async () => { + await client.publishJSON({ + api: { + name: "email", + provider: resend({ token: resendToken, batch: true }), + }, + headers: { + [header]: headerValue, + }, + method: "PUT", + body: [ + { + from: "Acme ", + to: ["foo@gmail.com"], + subject: "hello world", + html: "

it works!

", + }, + { + from: "Acme ", + to: ["bar@outlook.com"], + subject: "world hello", + html: "

it works!

", + }, + ], + }); + }, + responseFields: { + body: { messageId: "msgId" }, + status: 200, + }, + receivesRequest: { + method: "POST", + token: qstashToken, + url: "http://localhost:8080/v2/publish/https://api.resend.com/emails/batch", + body: [ + { + from: "Acme ", + to: ["foo@gmail.com"], + subject: "hello world", + html: "

it works!

", + }, + { + from: "Acme ", + to: ["bar@outlook.com"], + subject: "world hello", + html: "

it works!

", + }, + ], + headers: { + authorization: `Bearer ${qstashToken}`, + "upstash-forward-authorization": `Bearer ${resendToken}`, + "content-type": "application/json", + [`upstash-forward-${header}`]: headerValue, + "upstash-method": "PUT", }, }, }); diff --git a/src/client/api/email.ts b/src/client/api/email.ts index e74ceb7f..92df62c6 100644 --- a/src/client/api/email.ts +++ b/src/client/api/email.ts @@ -4,6 +4,7 @@ import type { EmailOwner, ProviderInfo } from "./types"; export class EmailProvider extends BaseProvider<"email", EmailOwner> { public readonly apiKind = "email"; public readonly batch: boolean; + public readonly method = "POST"; constructor(baseUrl: string, token: string, owner: EmailOwner, batch: boolean) { super(baseUrl, token, owner); diff --git a/src/client/api/llm.ts b/src/client/api/llm.ts index 71b46052..44a2496a 100644 --- a/src/client/api/llm.ts +++ b/src/client/api/llm.ts @@ -5,6 +5,7 @@ import { updateWithAnalytics } from "./utils"; export class LLMProvider extends BaseProvider<"llm", LLMOwner> { public readonly apiKind = "llm"; public readonly organization?: string; + public readonly method = "POST"; constructor(baseUrl: string, token: string, owner: TOwner, organization?: string) { super(baseUrl, token, owner); diff --git a/src/client/api/types.ts b/src/client/api/types.ts index cb39dc27..37990247 100644 --- a/src/client/api/types.ts +++ b/src/client/api/types.ts @@ -1,3 +1,4 @@ +import type { HTTPMethods } from "../types"; import type { BaseProvider } from "./base"; export type ProviderInfo = { @@ -21,6 +22,10 @@ export type ProviderInfo = { * provider owner */ owner: Owner; + /** + * method to use in the request + */ + method: HTTPMethods; }; export type ApiKind = "llm" | "email"; diff --git a/src/client/api/utils.ts b/src/client/api/utils.ts index 58385e84..01841b57 100644 --- a/src/client/api/utils.ts +++ b/src/client/api/utils.ts @@ -38,6 +38,7 @@ export const getProviderInfo = ( route: finalProvider.getRoute(), appendHeaders: finalProvider.getHeaders(parameters), owner: finalProvider.owner, + method: finalProvider.method, }; return finalProvider.onFinish(providerInfo, parameters); @@ -93,7 +94,7 @@ export const processApi = ( return request; } - const { url, appendHeaders, owner } = getProviderInfo(request.api, upstashToken); + const { url, appendHeaders, owner, method } = getProviderInfo(request.api, upstashToken); if (request.api.name === "llm") { const callback = request.callback; @@ -103,6 +104,7 @@ export const processApi = ( return { ...request, + method: request.method ?? method, headers: safeJoinHeaders(headers, appendHeaders), ...(owner === "upstash" && !request.api.analytics ? { api: { name: "llm" }, url: undefined, callback } @@ -111,6 +113,7 @@ export const processApi = ( } else { return { ...request, + method: request.method ?? method, headers: safeJoinHeaders(headers, appendHeaders), url, api: undefined,