Skip to content

Commit

Permalink
Merge branch 'master' into reset-abort
Browse files Browse the repository at this point in the history
  • Loading branch information
igorkamyshev committed Apr 10, 2024
2 parents 42935bc + e011508 commit 61d841a
Show file tree
Hide file tree
Showing 8 changed files with 146 additions and 4 deletions.
5 changes: 5 additions & 0 deletions .changeset/fuzzy-cooks-join.md
@@ -0,0 +1,5 @@
---
"@farfetched/core": patch
---

Fix bug with extra fires of `finished.failure` in case of failed contract application with `retry` operator
54 changes: 54 additions & 0 deletions CONTRIBUTING.md
@@ -0,0 +1,54 @@
# Contributing to Farfetched

👍🎉 First off, thanks for taking the time to contribute! 🎉👍

The following is a set of guidelines for contributing to Farfetched. These are mostly guidelines, not rules. Use your best judgment, and feel free to propose changes to this document in a pull request.

## Basics

### Architecture review

Any new feature or change should be discussed with maintainers via issue with RFC. It will help to avoid unnecessary work and make sure that the feature will be accepted. RFCs are accepted in English and Russian languages.

After discussion, RFC should be transformed to an ADR in the `apps/website/adr` directory. It will be a part of the documentation. ADRs are accepted in English only.

### Documentation first

Any new feature or change should be documented. It is a part of the feature itself. Documentation should be placed in the `apps/website/docs` directory. It is written in Markdown and accepted in English only.

### Tests second

Any new feature or change should be covered by tests. It is a part of the feature itself. Tests have to cover runtime implementation **and** TypeScript types declarations. Farfetched uses [Vitest](https://vitest.dev/) as a test runner, please refer to its documentation for more information.

### Implementation last

After documentation and tests are ready, you can start implementing the feature. Please ensure that your implementation is passing all checks. You can run them via `pnpm ci:local` to get quicker feedback than CI.

### Changeset

After implementation is ready, you have to fill in changes via `pnpm changeset`. It will create a new changeset in the `.changeset` directory. Changesets are accepted in English only. It will be used for versioning and changelog generation.

## How to create a good API

While writing RFC, keep in mind that the new API (or changes in the existing API) should be consistent with the following principles:

1. Solve a real problem. The new feature should solve a real problem that is faced by developers. You have to provide a clear example of the problem and how the new feature solves it.
2. Be consistent. The new feature should be consistent with the existing API. It should not introduce new concepts or patterns that are not used in the existing API unless it is necessary.
3. Simple solutions for simple problems, complex solutions for complex problems. Not vice versa.

Read existing [ADRs](https://farfetched.pages.dev/adr/) to get an understanding of the existing API and principles.

## How to create a good implementation

While writing code, keep in mind that Farfetched is a library that is focused on developer and user experiences. Moreover, Farfetched is based on [Effector](https://effector.dev/) and has to be consistent with its principles and APIs. There are some tips that will help you to create a good implementation:

1. Fork API is a key. Any feature should support Fork API.
2. Performance is important. Avoid unnecessary computations and slow algorithms.
3. TypeScript is a must. Ensure that your implementation is typed correctly.
4. Declarative API > Imperative API. Users should be able to express the logic of a computation without describing its control flow using your feature.

## General advice

- Respect [`.nvmrc`](./.nvmrc) file. We cannot guarantee that Farfetched will work with other Node.js versions.
- Respect `packageManager` field in [`package.json`](./package.json).
- Keep PRs on-topic. If you want to introduce two unrelated changes, please create two separate PRs.
4 changes: 4 additions & 0 deletions README.md
Expand Up @@ -21,6 +21,10 @@ Continue reading about Farfetched in the [documentation](https://farfetched.page

Repository contains [several showcases](./apps/) of Farfetched usage. To start playing with them, clone repository and run `pnpm install && pnpm run --filter NAME dev` in the root directory, where `NAME` is the name of the showcase.

## Contributing

If you want to contribute to Farfetched, please read the [CONTRIBUTING.md](./CONTRIBUTING.md) file first.

## Maintains

### Getting started
Expand Down
3 changes: 2 additions & 1 deletion package.json
Expand Up @@ -2,7 +2,8 @@
"packageManager": "pnpm@7.28.0",
"scripts": {
"format:check": "pnpm prettier {packages,apps}/**/*.{ts,tsx,md,vue} --check",
"format:apply": "pnpm prettier {packages,apps}/**/*.{ts,tsx,md,vue} --write"
"format:apply": "pnpm prettier {packages,apps}/**/*.{ts,tsx,md,vue} --write",
"ci:local": "pnpm run format:check && pnpm run -r build && pnpm run -r test:run && pnpm run -r size && pnpm run -r publint && pnpm run -r typelint"
},
"devDependencies": {
"@algolia/client-search": "^4.14.2",
Expand Down
37 changes: 35 additions & 2 deletions packages/core/src/remote_operation/create_remote_operation.ts
Expand Up @@ -153,6 +153,11 @@ export function createRemoteOperation<
error: Error | InvalidDataError;
meta: ExecutionMeta;
}>();
const failedIgnoreSuppression = createEvent<{
params: Params;
error: Error | InvalidDataError;
meta: ExecutionMeta;
}>();
const aborted = createEvent<{
params: Params;
meta: ExecutionMeta;
Expand Down Expand Up @@ -284,7 +289,10 @@ export function createRemoteOperation<
fn: ({ params, result }) => ({
params: params.params,
result: result.result as Data,
meta: { stopErrorPropagation: false, stale: result.stale },
meta: {
stopErrorPropagation: result.stopErrorPropagation ?? false,
stale: result.stale,
},
}),
filter: $enabled,
target: applyContractFx,
Expand Down Expand Up @@ -374,6 +382,17 @@ export function createRemoteOperation<
target: failedNoFilters,
});

sample({
clock: applyContractFx.fail,
fn: ({ error, params }) => ({
error,
// Extract original params, it is params of params
params: params.params,
meta: params.meta,
}),
target: failedIgnoreSuppression,
});

sample({
clock: invalidDataRecieved,
filter: ({ meta }) => !meta.stopErrorPropagation,
Expand All @@ -388,6 +407,19 @@ export function createRemoteOperation<
target: failedNoFilters,
});

sample({
clock: invalidDataRecieved,
fn: ({ params, validation, meta, result }) => ({
params,
error: invalidDataError({
validationErrors: unwrapValidationResult(validation),
response: result,
}),
meta,
}),
target: failedIgnoreSuppression,
});

// Emit skip for disabling in-flight operation
sample({
clock: $enabled.updates,
Expand Down Expand Up @@ -464,6 +496,7 @@ export function createRemoteOperation<
pushData,
startWithMeta,
callObjectCreated,
failedIgnoreSuppression,
},
},
};
Expand All @@ -480,7 +513,7 @@ function createDataSourceHandlers<Params>(dataSources: DataSource<Params>[]) {
skipStale?: boolean;
meta: ExecutionMeta;
},
{ result: unknown; stale: boolean },
{ result: unknown; stale: boolean; stopErrorPropagation?: boolean },
{ stopErrorPropagation: boolean; error: unknown }
>({
handler: async ({ params, skipStale }) => {
Expand Down
5 changes: 5 additions & 0 deletions packages/core/src/remote_operation/type.ts
Expand Up @@ -123,6 +123,11 @@ export interface RemoteOperation<
pushError: EventCallable<Error>;
startWithMeta: EventCallable<{ params: Params; meta: ExecutionMeta }>;
callObjectCreated: Event<CallObject>;
failedIgnoreSuppression: Event<{
params: Params;
error: Error;
meta: ExecutionMeta;
}>;
} & ExtraLowLevelAPI;
experimentalAPI?: {
attach: <Source, NewParams>(config: {
Expand Down
35 changes: 35 additions & 0 deletions packages/core/src/retry/__tests__/retry.test.ts
Expand Up @@ -4,6 +4,8 @@ import { allSettled, createWatch, fork } from 'effector';
import { unknownContract } from '../../contract/unknown_contract';
import { createJsonMutation } from '../../mutation/create_json_mutation';
import { httpError } from '../../errors/create_error';
import { createJsonQuery } from '../../query/create_json_query';
import { fetchFx } from '../../fetch/fetch';
import { retry } from '../retry';

describe('retry', () => {
Expand Down Expand Up @@ -48,4 +50,37 @@ describe('retry', () => {

expect(onFailure).toBeCalledTimes(1);
});

test('does supress contract failure error as well, issue #459', async () => {
const q = createJsonQuery({
request: { url: 'https://api.salo.com', method: 'GET' },
response: {
contract: {
isData(v): v is unknown {
return false;
},
getErrorMessages(v) {
return ['Failed contract'];
},
},
},
});

retry(q, { times: 1, delay: 1 });

const scope = fork({
handlers: [[fetchFx, () => new Response('{"data": "ok"}')]],
});

const failedListener = vi.fn();
createWatch({ unit: q.finished.failure, scope, fn: failedListener });

const startedListener = vi.fn();
createWatch({ unit: q.started, scope, fn: startedListener });

await allSettled(q.start, { scope });

expect(startedListener).toBeCalledTimes(2 /* Initial and retry */);
expect(failedListener).toBeCalledTimes(1 /* Only latest failure */);
});
});
7 changes: 6 additions & 1 deletion packages/core/src/retry/retry.ts
Expand Up @@ -160,6 +160,11 @@ export function retry<
const originalFx =
operation.__.lowLevelAPI.dataSourceRetrieverFx.use.getCurrent();

sample({
clock: operation.__.lowLevelAPI.failedIgnoreSuppression,
target: failed,
});

operation.__.lowLevelAPI.dataSourceRetrieverFx.use(
attach({
source: { supressError: $supressError, partialFilter: $partialFilter },
Expand All @@ -182,7 +187,7 @@ export function retry<
try {
const result = await originalFx(opts);

return result;
return { ...result, stopErrorPropagation: supressError };
} catch (error: any) {
const failInfo = {
params: opts.params,
Expand Down

0 comments on commit 61d841a

Please sign in to comment.