Skip to content

fix(client): parse Presto JSON response with custom reviver and BigInts #25

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

Merged
merged 8 commits into from
Dec 20, 2024
2 changes: 1 addition & 1 deletion .nvmrc
Original file line number Diff line number Diff line change
@@ -1 +1 @@
18.17.1
21.0.0
14 changes: 13 additions & 1 deletion presto-client/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
QueryInfo,
Table,
} from './types'
import { parseWithBigInts, isJsonParseContextAvailable } from './utils'

export class PrestoClient {
private baseUrl: string
Expand Down Expand Up @@ -270,7 +271,7 @@ export class PrestoClient {
throw new Error(`Query failed: ${JSON.stringify(await response.text())}`)
}

const prestoResponse = (await response.json()) as PrestoResponse
const prestoResponse = (await this.prestoConversionToJSON({ response })) as PrestoResponse
if (!prestoResponse) {
throw new Error(`Query failed with an empty response from the server.`)
}
Expand Down Expand Up @@ -334,6 +335,17 @@ export class PrestoClient {
method,
})
}

private async prestoConversionToJSON({ response }: { response: Response }): Promise<unknown> {
const text = await response.text()
if (isJsonParseContextAvailable()) {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore JSON.parse with a 3 argument reviver is a stage 3 proposal with some support, allow it here.
return JSON.parse(text, parseWithBigInts)
} else {
return JSON.parse(text)
}
}
}

export default PrestoClient
39 changes: 39 additions & 0 deletions presto-client/src/utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/**
* Parses a JSON including bigger numbers into BigInts
* @param _ Key
* @param value Parsed value
* @param context Context with source text
* @returns Parsed object with BigInts where required
*/
export function parseWithBigInts(_: string, value: unknown, { source }: { source: string }) {
// Ignore non-numbers
if (typeof value !== 'number') return value

// If not an integer, use the value
// TODO: Check if Presto can return floats that could also lose precision
if (!Number.isInteger(value)) return value

// If number is a safe integer, we can use it
if (Number.isSafeInteger(value)) return value

return BigInt(source)
}

/**
* Checks if JSON.parse reviver callback has a context parameter
* See also:
* - https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/parse#browser_compatibility
* - https://github.com/tc39/proposal-json-parse-with-source
*
* This implementation is based on suggestion here:
* - https://github.com/tc39/proposal-json-parse-with-source/issues/40
*/
export function isJsonParseContextAvailable() {
let contextAvailable
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
JSON.parse('"x"', function (key, value, x) {
contextAvailable = typeof x !== 'undefined'
})
return contextAvailable
}
Loading