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.

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

Preserve error types during translation #11834

Merged
merged 1 commit into from
Apr 28, 2020

Conversation

xelan
Copy link
Contributor

@xelan xelan commented Apr 22, 2020

Q A
Branch? master
Bug fix? yes
New feature? yes
Deprecations? no
Tickets Fix #11658
License Apache License 2.0

By preserving the exception type, more fine-grained error handling can be performed via client-side logic (e.g. redirect to a search page if a PDF is not found, or to a ticket system in case of invalid PDF files).

The original exception type and its properties (e.g. status code) are preserved by cloning the error object and assigning the translated message. Note that only a shallow clone is performed for simplicity.

Fixes #11658

@xelan xelan force-pushed the feature/preserve-error-types branch from ae8522c to 7ce370b Compare April 22, 2020 12:25
Copy link
Collaborator

@Snuffleupagus Snuffleupagus left a comment

Choose a reason for hiding this comment

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

This seem like way too much code/complexity for an edge-case, and in a previous PR (see #11689) the consensus was that if this should be fixed then simply re-throwing the original exception would be preferable.

@xelan
Copy link
Contributor Author

xelan commented Apr 22, 2020

Ok, if the possible change of the original exception outside of pdf.js is acceptable, I can just rethrow the exception with the translated message, without any cloning 😄

@xelan xelan force-pushed the feature/preserve-error-types branch from 7ce370b to d614534 Compare April 22, 2020 12:48
@xelan
Copy link
Contributor Author

xelan commented Apr 22, 2020

I've pushed the updated, simplified version which rethrows the original exception.

web/app.js Outdated Show resolved Hide resolved
By preserving the exception type, more fine-grained error handling can be performed via client-side logic (e.g. redirect to a search page if a PDF is not found, or to a ticket system in case of invalid PDF files).

The original exception is now re-thrown.

Fixes mozilla#11658
@xelan xelan force-pushed the feature/preserve-error-types branch from d614534 to f5fd24a Compare April 28, 2020 07:36
@timvandermeij
Copy link
Contributor

/botio-linux preview

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

Command cmd_preview from @timvandermeij received. Current queue size: 0

Live output at: http://54.67.70.0:8877/4ab244e9adc073f/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/4ab244e9adc073f/output.txt

Total script time: 3.34 mins

Published

@timvandermeij timvandermeij merged commit b6f69d4 into mozilla:master Apr 28, 2020
@timvandermeij
Copy link
Contributor

Let's do this. Thank you for your contribution!

@xelan
Copy link
Contributor Author

xelan commented Apr 29, 2020

Let's do this. Thank you for your contribution!

Great 👍 You're welcome. Thanks for the feedback and the quick and professional handling.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Preserve error types (e.g. InvalidPDFException, MissingPDFException)
4 participants