-
-
Notifications
You must be signed in to change notification settings - Fork 48
WIP: Improve Result class #555
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -127,7 +127,7 @@ const PARSE_ERR_CODES = { | |
|
|
||
| class FrameParser { | ||
| static parse(buffer) { | ||
| if (buffer.length < 2) return Result.empty(); | ||
| if (buffer.length < 2) return Result.ok(null); | ||
|
|
||
| const fin = (buffer[0] & FIN_MASK) !== 0; | ||
| const rsv = buffer[0] & RSV_MASK; | ||
|
|
@@ -137,7 +137,7 @@ class FrameParser { | |
| let offset = 2; | ||
|
|
||
| if (rsv !== 0) { | ||
| return Result.from( | ||
| return Result.fail( | ||
| new ParseError( | ||
| PARSE_ERR_CODES.PROTOCOL_ERROR_RSV, | ||
| 'RSV bits must be 0', | ||
|
|
@@ -146,17 +146,17 @@ class FrameParser { | |
| } | ||
|
|
||
| if (length === LEN_16_BIT) { | ||
| if (buffer.length < offset + 2) return Result.empty(); | ||
| if (buffer.length < offset + 2) return Result.ok(null); | ||
| length = buffer.readUInt16BE(offset); | ||
| offset += 2; | ||
| } else if (length === LEN_64_BIT) { | ||
| if (buffer.length < offset + 8) return Result.empty(); | ||
| if (buffer.length < offset + 8) return Result.ok(null); | ||
| const high = buffer.readUInt32BE(offset); | ||
| const low = buffer.readUInt32BE(offset + 4); | ||
| offset += 8; | ||
| const isSafeHigh = (high & MAX_SAFE_HIGH_MASK) === 0; | ||
| if (!isSafeHigh) { | ||
| return Result.from( | ||
| return Result.fail( | ||
| new ParseError( | ||
| PARSE_ERR_CODES.MESSAGE_TOO_BIG, | ||
| 'Payload length exceeds MAX_SAFE_INTEGER', | ||
|
|
@@ -168,36 +168,36 @@ class FrameParser { | |
|
|
||
| let mask; | ||
| if (masked) { | ||
| if (buffer.length < offset + 4) return Result.empty(); | ||
| if (buffer.length < offset + 4) return Result.ok(null); | ||
| mask = buffer.subarray(offset, offset + 4); | ||
| offset += 4; | ||
| } | ||
|
|
||
| if (buffer.length < offset + length) return Result.empty(); | ||
| if (buffer.length < offset + length) return Result.ok(null); | ||
| const payload = buffer.subarray(offset, offset + length); | ||
| const frame = new Frame(fin, opcode, masked, payload, mask, rsv); | ||
| return Result.from({ frame, bytesUsed: offset + length }); | ||
| return Result.ok({ frame, bytesUsed: offset + length }); | ||
| } | ||
|
|
||
| static checkControlFrame(frame) { | ||
| const { fin, opcode, payload } = frame; | ||
| if (!CONTROL_OPCODES.has(opcode) || !fin) { | ||
| return Result.from( | ||
| if (!CONTROL_OPCODES.includes(opcode) || !fin) { | ||
| return Result.fail( | ||
| new ParseError(PARSE_ERR_CODES.PROTOCOL_ERROR_COMMON, 'Protocol error'), | ||
| ); | ||
|
Comment on lines
+185
to
187
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use intermediate identifiers |
||
| } | ||
| if (payload.length > 125) { | ||
| return Result.from( | ||
| return Result.fail( | ||
| new ParseError( | ||
| PARSE_ERR_CODES.PROTOCOL_ERROR_CTRL_TOO_LONG, | ||
| 'Control frame too long', | ||
| ), | ||
| ); | ||
| } | ||
| if (opcode === OPCODES.CLOSE) { | ||
| if (payload.length === 0) return Result.from(true); | ||
| if (payload.length === 0) return Result.ok(true); | ||
| if (payload.length === 1) { | ||
| return Result.from( | ||
| return Result.fail( | ||
| new ParseError( | ||
| PARSE_ERR_CODES.PROTOCOL_ERROR_COMMON, | ||
| 'Protocol error', | ||
|
|
@@ -207,42 +207,42 @@ class FrameParser { | |
| const code = payload.readUInt16BE(0); | ||
| const reason = payload.subarray(2); | ||
| if (!isValidCloseCode(code)) { | ||
| return Result.from( | ||
| return Result.fail( | ||
| new ParseError( | ||
| PARSE_ERR_CODES.PROTOCOL_ERROR_COMMON, | ||
| `Invalid close code: ${code}`, | ||
| ), | ||
| ); | ||
| } | ||
| if (!isValidUTF8(reason)) { | ||
| return Result.from( | ||
| return Result.fail( | ||
| new ParseError( | ||
| PARSE_ERR_CODES.INVALID_PAYLOAD, | ||
| 'Invalid UTF-8 in close reason', | ||
| ), | ||
| ); | ||
| } | ||
| } | ||
| return Result.from(true); | ||
| return Result.ok(true); | ||
| } | ||
|
|
||
| static checkDataFrame(frame) { | ||
| const { fin, opcode, payload } = frame; | ||
| if (!DATA_OPCODES.has(opcode)) { | ||
| return Result.from( | ||
| if (!DATA_OPCODES.includes(opcode)) { | ||
| return Result.fail( | ||
| new ParseError(PARSE_ERR_CODES.PROTOCOL_ERROR_COMMON, 'Protocol error'), | ||
| ); | ||
| } | ||
| const isText = opcode === OPCODES.TEXT; | ||
| if (isText && fin && !isValidUTF8(payload)) { | ||
| return Result.from( | ||
| return Result.fail( | ||
| new ParseError( | ||
| PARSE_ERR_CODES.INVALID_PAYLOAD, | ||
| 'Invalid UTF-8 in text frame', | ||
| ), | ||
| ); | ||
| } | ||
| return Result.from(true); | ||
| return Result.ok(true); | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,24 +1,38 @@ | ||
| 'use strict'; | ||
|
|
||
| const NO_DEFAULT = Symbol('NoDefault'); | ||
|
|
||
| class Result { | ||
| #value; | ||
| #error; | ||
| #value = null; | ||
| #error = null; | ||
|
|
||
| constructor(value = null, error = null) { | ||
| if (value !== null) this.#value = value; | ||
| if (error !== null) this.#error = error; | ||
| } | ||
|
|
||
| constructor({ value, error }) { | ||
| this.#value = value; | ||
| this.#error = error; | ||
| static ok(value = null) { | ||
| return new Result(value, null); | ||
| } | ||
|
|
||
| static from(input) { | ||
| const res = | ||
| input instanceof globalThis.Error | ||
| ? { value: null, error: input } | ||
| : { value: input, error: null }; | ||
| return new this(res); | ||
| static fail(error) { | ||
| return new Result(null, error); | ||
| } | ||
|
Comment on lines
+18
to
20
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. need an error by default |
||
|
|
||
| static empty() { | ||
| return new this({ value: null, error: null }); | ||
| static from(fn) { | ||
| try { | ||
| return Result.ok(fn()); | ||
| } catch (error) { | ||
| return Result.fail(error); | ||
| } | ||
| } | ||
|
|
||
| static async fromAsync(fn) { | ||
| try { | ||
| return Result.ok(await fn()); | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. await at separate line |
||
| } catch (error) { | ||
| return Result.fail(error); | ||
| } | ||
| } | ||
|
|
||
| get value() { | ||
|
|
@@ -28,6 +42,21 @@ class Result { | |
| get error() { | ||
| return this.#error; | ||
| } | ||
|
|
||
| get ok() { | ||
| return this.#error === null; | ||
| } | ||
|
|
||
| unwrap(defaultValue = NO_DEFAULT) { | ||
| if (this.#error === null) return this.#value; | ||
| if (defaultValue === NO_DEFAULT) throw this.#error; | ||
| return defaultValue; | ||
| } | ||
|
|
||
| map(fn) { | ||
| if (this.#error !== null) return this; | ||
| return Result.from(() => fn(this.#value)); | ||
| } | ||
| } | ||
|
|
||
| module.exports = { Result }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,109 @@ | ||
| 'use strict'; | ||
|
|
||
| const { test } = require('node:test'); | ||
| const assert = require('node:assert/strict'); | ||
|
|
||
| const { Result } = require('../lib/websocket/result.js'); | ||
|
|
||
| test('Result.ok: creates success result with given value', () => { | ||
| const r = Result.ok(42); | ||
| assert.equal(r.value, 42); | ||
| assert.equal(r.error, null); | ||
| assert.equal(r.ok, true); | ||
| }); | ||
|
|
||
| test('Result.ok: defaults to null value', () => { | ||
| const r = Result.ok(); | ||
| assert.equal(r.value, null); | ||
| assert.equal(r.error, null); | ||
| assert.equal(r.ok, true); | ||
| }); | ||
|
|
||
| test('Result.fail: creates error result', () => { | ||
| const err = new Error('boom'); | ||
| const r = Result.fail(err); | ||
| assert.equal(r.value, null); | ||
| assert.strictEqual(r.error, err); | ||
| assert.equal(r.ok, false); | ||
| }); | ||
|
|
||
| test('Result.from: wraps a function return value', () => { | ||
| const r = Result.from(() => ({ code: 1000, reason: 'ok' })); | ||
| assert.deepEqual(r.value, { code: 1000, reason: 'ok' }); | ||
| assert.equal(r.error, null); | ||
| assert.equal(r.ok, true); | ||
| }); | ||
|
|
||
| test('Result.from: catches thrown errors', () => { | ||
| const err = new Error('boom'); | ||
| const r = Result.from(() => { | ||
| throw err; | ||
| }); | ||
| assert.equal(r.value, null); | ||
| assert.strictEqual(r.error, err); | ||
| assert.equal(r.ok, false); | ||
| }); | ||
|
|
||
| test('Result.fromAsync: wraps an async return value', async () => { | ||
| const r = await Result.fromAsync(async () => 99); | ||
| assert.equal(r.value, 99); | ||
| assert.equal(r.error, null); | ||
| assert.equal(r.ok, true); | ||
| }); | ||
|
|
||
| test('Result.fromAsync: catches rejected promises', async () => { | ||
| const err = new Error('async boom'); | ||
| const r = await Result.fromAsync(async () => { | ||
| throw err; | ||
| }); | ||
| assert.equal(r.value, null); | ||
| assert.strictEqual(r.error, err); | ||
| assert.equal(r.ok, false); | ||
| }); | ||
|
|
||
| test('Result.unwrap: returns value when ok', () => { | ||
| assert.equal(Result.ok(7).unwrap(), 7); | ||
| }); | ||
|
|
||
| test('Result.unwrap: throws error when failed', () => { | ||
| const err = new Error('fail'); | ||
| assert.throws( | ||
| () => Result.fail(err).unwrap(), | ||
| (e) => e === err, | ||
| ); | ||
| }); | ||
|
|
||
| test('Result.unwrap: returns defaultValue when failed', () => { | ||
| const r = Result.fail(new Error('fail')); | ||
| assert.equal(r.unwrap('default'), 'default'); | ||
| }); | ||
|
|
||
| test('Result.map: transforms value when ok', () => { | ||
| const r = Result.ok(3).map((v) => v * 2); | ||
| assert.equal(r.value, 6); | ||
| assert.equal(r.ok, true); | ||
| }); | ||
|
|
||
| test('Result.map: passes error through unchanged', () => { | ||
| const err = new Error('fail'); | ||
| const r = Result.fail(err).map((v) => v * 2); | ||
| assert.strictEqual(r.error, err); | ||
| assert.equal(r.ok, false); | ||
| }); | ||
|
|
||
| test('Result.map: catches error thrown inside mapper', () => { | ||
| const mapErr = new Error('map fail'); | ||
| const r = Result.ok(1).map(() => { | ||
| throw mapErr; | ||
| }); | ||
| assert.strictEqual(r.error, mapErr); | ||
| assert.equal(r.ok, false); | ||
| }); | ||
|
|
||
| test('Result: value and error getters are read-only', () => { | ||
| const r = Result.ok(42); | ||
| assert.equal(r.value, 42); | ||
| assert.throws(() => { | ||
| r.value = 99; | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is null by default so do not need to pass null, here and in all places like this