Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: ошибка 500 при сабмите формы отправки пьесы #488

Merged
merged 4 commits into from
Sep 20, 2023
Merged

fix: ошибка 500 при сабмите формы отправки пьесы #488

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e4413ce
fix: ошибка 500 при сабмите формы отправки пьесы
Aleksey-dev-crt Sep 13, 2023
4d49f55
fix: правки после ревью
Aleksey-dev-crt Sep 14, 2023
6c5013f
fix: правки после ревью
Aleksey-dev-crt Sep 14, 2023
4088512
fix: правки после ревью
Aleksey-dev-crt Sep 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions src/pages/form/form.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,16 +152,17 @@ const Participation = () => {

const handleSubmit = async (event: React.FormEvent<HTMLFormElement>) => {
event.preventDefault();
if (canSubmit) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Все хорошо, только можно упростить убрав лишний return

if (!canSubmit) return;

и router.push('/form/success') убрать в try

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

router.push('/form/success') убирать в try не вижу смысла, тогда редирект на страницу success произойдет, даже если запрос выполнится с ошибкой, можно добавить в блок finally, но тогда придется добавлять условие на случай ошибки. Сейчас реализовано корректно, на мой взгляд, т. к. в блоке catch есть return, т. е. если запрос выполнится с ошибкой редиректа на страницу success не будет.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"тогда редирект на страницу success произойдет, даже если запрос выполнится с ошибкой" - если запрос выполняется с ошибкой, то он должен попадать в блок catch

"блоке catch есть return, т. е. если запрос выполнится с ошибкой редиректа на страницу success не будет" - ты же говоришь, что при ошибке запроса произойдет редирект на success в блоке catch!? если я правильно тебя понял, то тут противоречия

эти два блока кода, абсолютно одинаковы по функциональности, но второй вариант предпочтительнее, читаемостью и структурой

`const handleSubmit = async (event: React.FormEvent) => {
event.preventDefault();
if (canSubmit) {
try {
await postParticipation(form.values);
} catch (error) {
handleSubmitError(error);

    return;
  }

  router.push('/form/success');
}

};

const handleSubmit = async (event: React.FormEvent<HTMLFormElement>) => {
event.preventDefault();
if (!canSubmit) return;
try {
    await postParticipation(form.values);
    router.push('/form/success');
  } catch (error) {
    handleSubmitError(error);
  }
}

};`

Copy link
Collaborator

@AlMkin AlMkin Sep 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Как ты проверял? опиши свои кейсы

"а как появится ошибка, если не выполнится блок try?" - этот момент не очень понял

Copy link
Collaborator Author

@Aleksey-dev-crt Aleksey-dev-crt Sep 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

сорян, да действительно я ошибался, сейчас еще раз перепроверил... Видимо в прошлый раз строчку router.push('/form/success'); ставил выше запроса и не обратил на это внимания, все поправил, изменения запушил

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

еще раз извиняюсь, не внимательный)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Я понял почему у меня отрабатывал router.push('/form/success'); в блоке try, даже, если запрос выполнялся с ошибкой, router.push у меня стоял после запроса, но в handleResponse в fetcher.ts был блок try catch и в catch было return err из-за этого в компоненте form.tsx при сабмите формы я вообще не попадал в блок catch, не важно с ошибкой выполнился запрос или нет...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

`async function handleResponse(response: Response) {
let payload;
try {
payload = await response.json();
} catch (err) {
// Если возникла ошибка при попытке разобрать JSON, устанавливаем payload в пустой объект
payload = {};
}

if (!response.ok) {
throw new HttpRequestError({ statusCode: response.status, payload });
}

return payload as T;
}`

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В общем так как if (!response.ok) {
throw new HttpRequestError({ statusCode: response.status, payload });
} это ошибка выбраcывалась в блоке try, поэтому оно дальше попадало в catch который в handleResponse и дальше просто return error, которое кидало в try handleSubmit

Вот сверху вариант должен работать правильно, проверяй

try {
await postParticipation(form.values);
} catch (error) {
handleSubmitError(error);

try {
await postParticipation(form.values);
} catch (error) {
handleSubmitError(error);
return;
}

return;
router.push('/form/success');
}

router.push('/form/success');
};

const canSubmit = !form.nonFieldError
Expand Down Expand Up @@ -246,6 +247,7 @@ const Participation = () => {
<PhoneNumberInput
value={form.values.phoneNumber}
errorText={form.touched.phoneNumber ? form.errors.phoneNumber : ''}
onChange={(value) => form.setFieldValue('phoneNumber', '+7' + value)}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Будет лучше если вынести в отдельную функцию - в какой-нить handle...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ну тут я по аналогии со всеми остальными инпутами сделал, для всех остальных инпутов тогда тоже отдельные хендлеры прописать?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

да, давай

/>
</FormField>
</Form.Field>
Expand Down